From 2bc819e842a2e1cba31ce6cbc05d8f76e2c356d3 Mon Sep 17 00:00:00 2001 From: tdwd Date: Sat, 19 Sep 2026 18:47:27 +0200 Subject: [PATCH 1/2] state: stop one cathode instance erasing another's Session titles were lost and sessions went missing from the ctrl+r picker. /title wrote the store and printed its confirmation, then the next turn in any other window put the file back to a state that had never seen that row. Every state file is shared by every running cathode instance, and each one loaded the file at its own start. A write that rebuilt the file from that in-memory snapshot erased everything the other instances had written since. A row is only ever written by the instance whose session it is, so the loser of the race lost the row outright. Store-only rows (a codex thread) vanished from the picker; a claude session survived only because its JSONL is re-read from disk, which is why this read as a display bug. A write is now a read-modify-write under flock, and a read re-reads too, or the picker cannot list a session started in another window. flock rather than an O_EXCL lockfile because the kernel releases it when the process dies. The temp file is uniquely named: two instances sharing one sessions.jsonl.tmp wrote into the same file and renamed a half-finished one into place. Prompt history had the same bug at the same frequency. Appends are O_APPEND and safe, but maxHistoryEntries is small enough that an established history takes the cap-trim path on nearly every turn, so the whole-file rewrite was the write that ran. The trim now re-reads under the lock, and one function owns the cap rule for both startup and append. readJSONL and writeJSONL are shared, because the two stores had grown their own and only one had a unique temp name. settings.go still has the shape, recorded but not fixed: saveSettings writes the whole struct, so a read-modify-write cannot tell a stale field from a deliberate one without the call sites saying which one changed. --- history.go | 82 ++++--------------- history_test.go | 36 +++++++++ historyfile.go | 68 ++++++++++++++++ sessionfile.go | 68 ++++++++++++++++ sessionfile_test.go | 114 +++++++++++++++++++++++++++ sessionlabel.go | 61 +++++++++++++++ sessions.go | 183 +++++++++++++------------------------------ statefile.go | 83 ++++++++++++++++++++ statelock_unix.go | 36 +++++++++ statelock_windows.go | 16 ++++ 10 files changed, 552 insertions(+), 195 deletions(-) create mode 100644 historyfile.go create mode 100644 sessionfile.go create mode 100644 sessionfile_test.go create mode 100644 sessionlabel.go create mode 100644 statefile.go create mode 100644 statelock_unix.go create mode 100644 statelock_windows.go diff --git a/history.go b/history.go index 2a8603e..1f98ab1 100644 --- a/history.go +++ b/history.go @@ -4,9 +4,6 @@ package main import ( - "bufio" - "encoding/json" - "os" "strings" "sync" ) @@ -24,9 +21,13 @@ type promptEntry struct { // history is the Ctrl-Up/Down recall buffer. Persisted as JSONL at // $XDG_STATE_HOME/cathode/prompt-history.jsonl (or ~/.local/state/cathode/ -// when XDG_STATE_HOME is unset). Reads happen once at startup; appends touch -// only the tail in the common case, so it's safe to share across runs of the -// same session. +// when XDG_STATE_HOME is unset). +// +// entries is a cache of that file, not the store: every running cathode +// instance shares it, so the write rules in historyfile.go are what keep one +// window from erasing another's prompts. This comment used to claim appends +// "touch only the tail in the common case", which is what hid the fact that +// past the cap they did not — see trim. type history struct { mu sync.Mutex entries []promptEntry @@ -43,7 +44,9 @@ func openHistory() *history { return &history{} } h := &history{path: path} - h.load() + h.mu.Lock() + defer h.mu.Unlock() + h.load() // held-lock, like every other caller return h } @@ -53,54 +56,13 @@ func historyPath() (string, error) { return stateFilePath("prompt-history.jsonl") } -// load reads the JSONL and silently drops malformed lines (a previous crash -// mid-write shouldn't break recall). If the file is over the cap, we rewrite it -// trimmed — opencode does the same self-heal. -func (h *history) load() { - f, err := os.Open(h.path) - if err != nil { - return - } - defer f.Close() - sc := bufio.NewScanner(f) - sc.Buffer(make([]byte, 0, 64*1024), 1024*1024) - var lines []promptEntry - for sc.Scan() { - var e promptEntry - if err := json.Unmarshal(sc.Bytes(), &e); err == nil && e.Input != "" { - lines = append(lines, e) - } - } - if len(lines) > maxHistoryEntries { - lines = lines[len(lines)-maxHistoryEntries:] - h.entries = lines - h.rewrite() - return - } - h.entries = lines -} - -// rewrite atomically replaces the file with the in-memory entries. Only used -// for the cap-trim self-heal, never on the hot path. -func (h *history) rewrite() { - if h.path == "" { - return - } - tmp := h.path + ".tmp" - f, err := os.Create(tmp) - if err != nil { - return - } - for _, e := range h.entries { - b, _ := json.Marshal(e) - _, _ = f.Write(append(b, '\n')) - } - _ = f.Close() - _ = os.Rename(tmp, h.path) -} - // Append records a new prompt. Adjacent duplicates are dropped (re-sending the // same prompt three times leaves one entry). Resets the walk cursor to live. +// +// "Adjacent" is now adjacent in the shared file, not in this window's own +// entries, because load reads what every window appended. So sending a prompt +// another window just sent records nothing new — the history already ends with +// that line, which is what the rule was always for. func (h *history) Append(input string) { if strings.TrimSpace(input) == "" { return @@ -113,21 +75,11 @@ func (h *history) Append(input string) { } h.entries = append(h.entries, promptEntry{Input: input}) h.cursor = 0 - if len(h.entries) > maxHistoryEntries { - h.entries = h.entries[len(h.entries)-maxHistoryEntries:] - h.rewrite() - return - } if h.path == "" { return } - f, err := os.OpenFile(h.path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) - if err != nil { - return - } - defer f.Close() - b, _ := json.Marshal(promptEntry{Input: input}) - _, _ = f.Write(append(b, '\n')) + h.appendLine(input) + h.load() // re-read: the file also holds what the other windows appended } // Rewind puts the walk cursor back at live. Call it when the prompt is cleared diff --git a/history_test.go b/history_test.go index 5668ddf..c3da26d 100644 --- a/history_test.go +++ b/history_test.go @@ -4,6 +4,7 @@ package main import ( + "fmt" "path/filepath" "testing" ) @@ -36,6 +37,41 @@ func TestHistoryAppendCapsAtMax(t *testing.T) { } } +// The prompt-history file is shared by every cathode instance, and the cap is +// small enough that an established history trims on nearly every turn. So the +// trim must re-read the file rather than rebuild it from one instance's memory, +// or each window erases the other's prompts as fast as they are typed. +func TestHistoryKeepsWhatAnotherInstanceAppended(t *testing.T) { + path := filepath.Join(t.TempDir(), "prompt-history.jsonl") + a, b := &history{path: path}, &history{path: path} + + // Fill past the cap so every further append takes the trim path. + for i := 0; i < maxHistoryEntries; i++ { + a.Append(fmt.Sprintf("a-%02d", i)) + } + b.load() // B starts up and sees A's history + a.Append("typed in window A") + b.Append("typed in window B") + + fresh := &history{path: path} + fresh.load() + var seenA, seenB bool + for _, e := range fresh.entries { + switch e.Input { + case "typed in window A": + seenA = true + case "typed in window B": + seenB = true + } + } + if !seenA || !seenB { + t.Errorf("window A kept = %v, window B kept = %v; want both", seenA, seenB) + } + if len(fresh.entries) > maxHistoryEntries { + t.Errorf("entries = %d, want the cap held at %d", len(fresh.entries), maxHistoryEntries) + } +} + // TestHistoryMoveWalksAndReturnsToLive pins the cursor semantics: Ctrl-Up // recalls the newest entry, again recalls the one before, Ctrl-Down walks back // toward live and finally clears the input. diff --git a/historyfile.go b/historyfile.go new file mode 100644 index 0000000..667fd83 --- /dev/null +++ b/historyfile.go @@ -0,0 +1,68 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/json" + "os" +) + +// ---- how prompt history reaches the disk ---- +// +// The file is shared with every other cathode instance, so a prompt is appended +// to it and never written as part of a rebuild of it. O_APPEND is what lets all +// of them write it with no lock and no lost line. +// +// The one write that does replace the whole file is the cap self-heal in load, +// and it re-reads under the lock first, for the reason sessionStore documents. + +// load brings the cache up to date with the file, and holds the file to the cap. +// Held-lock: callers must hold h.mu. +// +// It runs after every append, not only at startup, because the file is shared: a +// prompt typed in another window belongs to this window's recall too, exactly as +// it does after a restart. The cap is small enough that an established history +// reaches this self-heal on nearly every turn, which is why it must start from +// what is on disk — writing this instance's view instead erased every prompt the +// other windows had appended since it started. +func (h *history) load() { + if h.path == "" { + return + } + if unlock, err := lockState(h.path + ".lock"); err == nil { + defer unlock() + } + lines := readHistory(h.path) + if len(lines) > maxHistoryEntries { + lines = lines[len(lines)-maxHistoryEntries:] + writeJSONL(h.path, lines) + } + h.entries = lines +} + +// readHistory reads the prompts in the order they were sent. A record with no +// text is dropped — there is nothing to recall. +func readHistory(path string) []promptEntry { + var out []promptEntry + for _, e := range readJSONL[promptEntry](path) { + if e.Input != "" { + out = append(out, e) + } + } + return out +} + +// appendLine writes one prompt to the end of the file. Held-lock. +func (h *history) appendLine(input string) { + f, err := os.OpenFile(h.path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) + if err != nil { + return + } + defer f.Close() + b, err := json.Marshal(promptEntry{Input: input}) + if err != nil { + return + } + _, _ = f.Write(append(b, '\n')) +} diff --git a/sessionfile.go b/sessionfile.go new file mode 100644 index 0000000..9bf555d --- /dev/null +++ b/sessionfile.go @@ -0,0 +1,68 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +package main + +// ---- how the session store reaches the disk ---- +// +// The rules that make a shared file safe live here, together, because missing +// one of them is what lost the user's session titles (sessionStore documents +// what happened). Nothing outside this file may write the store. + +// refresh reloads the cache from the file. Held-lock: callers must hold s.mu. +// With no path there is no file to read, and the cache is the whole store. +func (s *sessionStore) refresh() { + if s.path == "" { + return + } + s.entries = readSessions(s.path) +} + +// readSessions reads the store, keyed by id. A record with no id is dropped: it +// cannot be resumed or matched to anything, and it would render as a ghost row. +// A later line for the same id wins, so a file that was appended to rather than +// replaced still reads as one record per session. +func readSessions(path string) map[string]sessionInfo { + out := map[string]sessionInfo{} + for _, e := range readJSONL[sessionInfo](path) { + if e.ID != "" { + out[e.ID] = e + } + } + return out +} + +// mutate applies fn to the store and writes the result. Every write goes +// through here; nothing else may write the file. +// +// The re-read inside the lock is the point: it carries through the rows another +// instance has written since this one loaded, instead of replacing them with a +// stale snapshot. +func (s *sessionStore) mutate(fn func(map[string]sessionInfo)) { + s.mu.Lock() + defer s.mu.Unlock() + + if s.path == "" { + fn(s.entries) // no file to share, so nothing to lock or re-read + return + } + // A lock we could not take is not a reason to drop what the user did. The + // read-modify-write still runs, which leaves the far narrower race that + // Windows has all the time (statelock_windows.go). + if unlock, err := lockState(s.path + ".lock"); err == nil { + defer unlock() + } + s.refresh() + fn(s.entries) + s.write() +} + +// write replaces the file with the cache. Held-lock, and called only from +// mutate, which has just re-read what this is about to replace. +func (s *sessionStore) write() { + rows := make([]sessionInfo, 0, len(s.entries)) + for _, e := range s.entries { + rows = append(rows, e) + } + writeJSONL(s.path, rows) +} diff --git a/sessionfile_test.go b/sessionfile_test.go new file mode 100644 index 0000000..7b0e190 --- /dev/null +++ b/sessionfile_test.go @@ -0,0 +1,114 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "path/filepath" + "sync" + "testing" + "time" +) + +// Several cathode instances run at once, each holding the store open. A write +// from one must not erase what another wrote after it loaded. +// +// This is the bug that lost session titles: /title printed its confirmation, +// and the next turn in any other window rebuilt the file from a map loaded at +// that process's start — which had never seen the row, so the row went. Every +// test before this one used a single store, which is the assumption that let it +// in. +func TestAWriteKeepsWhatAnotherInstanceWrote(t *testing.T) { + t.Setenv("XDG_STATE_HOME", t.TempDir()) + a, b := openSessionStore(), openSessionStore() // both start on an empty file + + a.Touch("sess-a", "", "/repo", "", "claude", time.Now()) + a.SetTitle("sess-a", "the one A named") + + // B has never heard of sess-a. Its own write must not take the file back to + // the state B loaded. + b.Touch("sess-b", "", "/repo", "", "claude", time.Now()) + + fresh := openSessionStore() + if got := fresh.entries["sess-a"].Title; got != "the one A named" { + t.Errorf("A's title = %q, want it to survive B's write", got) + } + if _, ok := fresh.entries["sess-b"]; !ok { + t.Error("B's session is missing from the file") + } +} + +// The picker must list a session started in another window. That means reading +// the file at the moment the list is built, not at process start. +func TestThePickerSeesASessionAnotherInstanceStarted(t *testing.T) { + t.Setenv("XDG_STATE_HOME", t.TempDir()) + mine, other := openSessionStore(), openSessionStore() + + other.Touch("sess-other", "", "/repo", "started elsewhere", "claude", time.Now()) + + var found bool + for _, e := range mine.All() { + if e.ID == "sess-other" { + found = true + } + } + if !found { + t.Error("All() did not see the other instance's session") + } +} + +// Concurrent writes from separate stores must lose nothing and corrupt nothing. +// The file is replaced wholesale on every write, so without the lock this both +// drops rows and can rename a half-written temp file into place. +func TestConcurrentWritesKeepEveryRow(t *testing.T) { + dir := t.TempDir() + t.Setenv("XDG_STATE_HOME", dir) + + const n = 12 + var wg sync.WaitGroup + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + s := openSessionStore() // a store per instance, as in production + id := fmt.Sprintf("sess-%02d", i) + s.Touch(id, "", "/repo", "", "claude", time.Now()) + s.SetTitle(id, fmt.Sprintf("title %02d", i)) + }(i) + } + wg.Wait() + + got := openSessionStore() + for i := 0; i < n; i++ { + id := fmt.Sprintf("sess-%02d", i) + e, ok := got.entries[id] + if !ok { + t.Errorf("%s is missing", id) + continue + } + if want := fmt.Sprintf("title %02d", i); e.Title != want { + t.Errorf("%s title = %q, want %q", id, e.Title, want) + } + } + + // A shared temp name is how a half-written file gets renamed into place, so + // the name must be unique — and nothing may be left behind either way. + if leftover, _ := filepath.Glob(filepath.Join(dir, "cathode", "*.tmp*")); len(leftover) > 0 { + t.Errorf("temp files were not cleaned up: %v", leftover) + } +} + +// A store with no resolvable path still records, in memory. main falls back to +// one when the state dir cannot be resolved, and a session that cannot be +// persisted must not also fail to appear in this process's own picker. +func TestAStoreWithNoFileStillRecords(t *testing.T) { + s := &sessionStore{entries: map[string]sessionInfo{}} + s.Touch("sess-1", "", "/repo", "first", "claude", time.Now()) + s.SetTitle("sess-1", "named") + + all := s.All() + if len(all) != 1 || all[0].Title != "named" { + t.Errorf("All() = %+v, want the one titled session", all) + } +} diff --git a/sessionlabel.go b/sessionlabel.go new file mode 100644 index 0000000..26dd01d --- /dev/null +++ b/sessionlabel.go @@ -0,0 +1,61 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "strings" + "time" +) + +// ---- how a session record reads on screen ---- + +// sessionLabelMax bounds what a session record persists as its label, and is +// deliberately well past any width the picker can render (pickerMaxWidth). +// +// It is a *storage* bound, not a display one: it stops a pasted essay becoming +// a row's label and bloating the store, and nothing more. Truncating for the +// screen is the picker's job, because only it knows the terminal's width — when +// this number did that job instead, a 64-character cut left a third of the row +// empty on a wide terminal. +const sessionLabelMax = 200 + +// truncFirst normalises a first prompt into a one-line session label. It is the +// row's title when no /title was set, so it is the part the list is read for. +func truncFirst(s string) string { + // trunc, not a byte slice. s[:61] cuts a multi-byte rune in half and emits + // invalid UTF-8, and a first prompt is prose — the same bug sysPromptSummary + // was already fixed for. + return trunc(strings.TrimSpace(strings.ReplaceAll(s, "\n", " ")), sessionLabelMax) +} + +// humanizeAge renders "5m ago" / "2h ago" / "3d ago" style relative times. +func humanizeAge(t time.Time) string { + if t.IsZero() { + return "—" + } + d := time.Since(t) + switch { + case d < time.Minute: + return "just now" + case d < time.Hour: + return fmt.Sprintf("%dm ago", int(d.Minutes())) + case d < 24*time.Hour: + return fmt.Sprintf("%dh ago", int(d.Hours())) + default: + return fmt.Sprintf("%dd ago", int(d.Hours()/24)) + } +} + +// short truncates an identifier to 8 chars (with "—" for empty), used for +// session IDs in picker rows, status bars, and sidebar headers. +func short(s string) string { + if s == "" { + return "—" + } + if len(s) > 8 { + return s[:8] + } + return s +} diff --git a/sessions.go b/sessions.go index 33458ae..617fd7c 100644 --- a/sessions.go +++ b/sessions.go @@ -4,12 +4,7 @@ package main import ( - "bufio" - "encoding/json" - "fmt" - "os" "sort" - "strings" "sync" "time" ) @@ -42,10 +37,21 @@ func sessionBackend(v string) string { return v } -// sessionStore is the on-disk session index, persisted as JSONL at -// $XDG_STATE_HOME/cathode/sessions.jsonl. The file is rewritten atomically on -// every Touch so it stays one line per session — simpler than a log-compaction -// step and the file stays small (one entry per session we've ever seen). +// sessionStore is the session index, persisted as JSONL at +// $XDG_STATE_HOME/cathode/sessions.jsonl, one line per session ever seen. +// +// The *file* is the state and entries is only a cache of it. That distinction is +// the whole design, and getting it wrong lost user data: several cathode +// instances run at once, and a write that rebuilt the file from a map loaded at +// process start erased every row another instance had written since. A row is +// only ever written by the instance whose session it is, so the loser of that +// race lost the row outright — titles vanished, and store-only sessions +// disappeared from the picker. It was invisible for as long as the store was +// tested one process at a time. +// +// So every write re-reads the file under an exclusive lock, and every read +// re-reads it too. The file holds one line per session, and a turn is nowhere +// near hot enough for that to cost anything. type sessionStore struct { mu sync.Mutex entries map[string]sessionInfo @@ -58,7 +64,9 @@ func openSessionStore() *sessionStore { return &sessionStore{entries: map[string]sessionInfo{}} } s := &sessionStore{entries: map[string]sessionInfo{}, path: path} - s.load() + s.mu.Lock() + defer s.mu.Unlock() + s.refresh() return s } @@ -66,73 +74,58 @@ func sessionsPath() (string, error) { return stateFilePath("sessions.jsonl") } -func (s *sessionStore) load() { - f, err := os.Open(s.path) - if err != nil { - return - } - defer f.Close() - sc := bufio.NewScanner(f) - sc.Buffer(make([]byte, 0, 64*1024), 1024*1024) - for sc.Scan() { - var e sessionInfo - if err := json.Unmarshal(sc.Bytes(), &e); err == nil && e.ID != "" { - s.entries[e.ID] = e - } - } -} - -// Touch upserts a session. Empty model/cwd/first don't overwrite existing -// values (so a follow-up Touch carrying only LastUsed preserves prior -// metadata). LastUsed is always bumped. // SetTitle names a session. An empty title clears it, so the picker falls back // to the first prompt — there is no separate "unset" verb to remember. func (s *sessionStore) SetTitle(id, title string) { if id == "" { return } - s.mu.Lock() - defer s.mu.Unlock() - cur, ok := s.entries[id] - if !ok { - // Titling a session the store has never seen would create a row with no - // cwd, model or timestamp, which then sorts and renders as a ghost. - return - } - cur.Title = title - s.entries[id] = cur - s.rewrite() // called with the lock held, the same as Touch + s.mutate(func(m map[string]sessionInfo) { + cur, ok := m[id] + if !ok { + // Titling a session the store has never seen would create a row with + // no cwd, model or timestamp, which then sorts and renders as a ghost. + return + } + cur.Title = title + m[id] = cur + }) } +// Touch upserts a session. Empty model/cwd/first don't overwrite existing +// values (so a follow-up Touch carrying only LastUsed preserves prior +// metadata). LastUsed is always bumped. func (s *sessionStore) Touch(id, model, cwd, first, backend string, now time.Time) { if id == "" { return } - s.mu.Lock() - defer s.mu.Unlock() - cur := s.entries[id] - cur.ID = id - if backend != "" { - cur.Backend = backend - } - if model != "" { - cur.Model = model - } - if cwd != "" { - cur.Cwd = cwd - } - if first != "" && cur.First == "" { - cur.First = first - } - cur.LastUsed = now - s.entries[id] = cur - s.rewrite() + s.mutate(func(m map[string]sessionInfo) { + cur := m[id] + cur.ID = id + if backend != "" { + cur.Backend = backend + } + if model != "" { + cur.Model = model + } + if cwd != "" { + cur.Cwd = cwd + } + if first != "" && cur.First == "" { + cur.First = first + } + cur.LastUsed = now + m[id] = cur + }) } -// All returns sessions sorted most-recent first. +// All returns sessions sorted most-recent first. It re-reads the file, because +// the picker has to list what the other instances recorded too — a session +// started in another window is exactly the one you came to the picker for. func (s *sessionStore) All() []sessionInfo { s.mu.Lock() defer s.mu.Unlock() + s.refresh() out := make([]sessionInfo, 0, len(s.entries)) for _, e := range s.entries { out = append(out, e) @@ -140,73 +133,3 @@ func (s *sessionStore) All() []sessionInfo { sort.Slice(out, func(i, j int) bool { return out[i].LastUsed.After(out[j].LastUsed) }) return out } - -// rewrite is held-lock; callers must hold s.mu. Atomic via tmp + rename so a -// crash mid-write can't leave the file half-truncated. -func (s *sessionStore) rewrite() { - if s.path == "" { - return - } - tmp := s.path + ".tmp" - f, err := os.Create(tmp) - if err != nil { - return - } - for _, e := range s.entries { - b, _ := json.Marshal(e) - _, _ = f.Write(append(b, '\n')) - } - _ = f.Close() - _ = os.Rename(tmp, s.path) -} - -// ---- presentation helpers (display formatting) ---- - -// sessionLabelMax bounds what a session record persists as its label, and is -// deliberately well past any width the picker can render (pickerMaxWidth). -// -// It is a *storage* bound, not a display one: it stops a pasted essay becoming -// a row's label and bloating the store, and nothing more. Truncating for the -// screen is the picker's job, because only it knows the terminal's width — when -// this number did that job instead, a 64-character cut left a third of the row -// empty on a wide terminal. -const sessionLabelMax = 200 - -// truncFirst normalises a first prompt into a one-line session label. It is the -// row's title when no /title was set, so it is the part the list is read for. -func truncFirst(s string) string { - // trunc, not a byte slice. s[:61] cuts a multi-byte rune in half and emits - // invalid UTF-8, and a first prompt is prose — the same bug sysPromptSummary - // was already fixed for. - return trunc(strings.TrimSpace(strings.ReplaceAll(s, "\n", " ")), sessionLabelMax) -} - -// humanizeAge renders "5m ago" / "2h ago" / "3d ago" style relative times. -func humanizeAge(t time.Time) string { - if t.IsZero() { - return "—" - } - d := time.Since(t) - switch { - case d < time.Minute: - return "just now" - case d < time.Hour: - return fmt.Sprintf("%dm ago", int(d.Minutes())) - case d < 24*time.Hour: - return fmt.Sprintf("%dh ago", int(d.Hours())) - default: - return fmt.Sprintf("%dd ago", int(d.Hours()/24)) - } -} - -// short truncates an identifier to 8 chars (with "—" for empty), used for -// session IDs in picker rows, status bars, and sidebar headers. -func short(s string) string { - if s == "" { - return "—" - } - if len(s) > 8 { - return s[:8] - } - return s -} diff --git a/statefile.go b/statefile.go new file mode 100644 index 0000000..76b4e94 --- /dev/null +++ b/statefile.go @@ -0,0 +1,83 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bufio" + "encoding/json" + "os" + "path/filepath" +) + +// ---- reading and replacing a JSONL state file ---- +// +// Both persisted stores are JSONL under $XDG_STATE_HOME/cathode, and both are +// shared by every running cathode instance. The two operations that has to get +// right live here rather than once per store, because a copy differing by one +// detail is how a shared file loses data quietly: the session store and the +// prompt history each grew their own, and only one of them ended up with a +// unique temp name. + +// maxStateLine bounds one record. A pasted prompt is the longest thing either +// store holds, and a longer line is dropped rather than growing the scanner +// without limit. +const maxStateLine = 1024 * 1024 + +// readJSONL parses one record per line. A missing file is an empty store, and a +// line that will not parse is skipped rather than failing the whole read — one +// record left half-written by a crash must not cost the user the rest. +func readJSONL[T any](path string) []T { + f, err := os.Open(path) + if err != nil { + return nil + } + defer f.Close() + sc := bufio.NewScanner(f) + sc.Buffer(make([]byte, 0, 64*1024), maxStateLine) + var out []T + for sc.Scan() { + var rec T + if json.Unmarshal(sc.Bytes(), &rec) == nil { + out = append(out, rec) + } + } + return out +} + +// writeJSONL replaces path with one JSON line per record. +// +// Atomic via a temp file and a rename, so a crash cannot leave the file +// half-truncated and a concurrent reader sees either the old file or the new +// one. The temp name is unique: two instances sharing one ".tmp" write +// into the same file and rename a half-finished one into place, which is the one +// way a whole store goes at once. +// +// Callers must hold the file lock (lockState) and must have re-read the file +// inside it. Replacing a file that several instances write means starting from +// what is on disk, not from a cache loaded at process start — see sessionStore +// for what that cost. +func writeJSONL[T any](path string, records []T) { + tmp, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".tmp-*") + if err != nil { + return + } + for _, r := range records { + b, err := json.Marshal(r) + if err != nil { + continue + } + if _, err := tmp.Write(append(b, '\n')); err != nil { + _ = tmp.Close() + _ = os.Remove(tmp.Name()) + return + } + } + if err := tmp.Close(); err != nil { + _ = os.Remove(tmp.Name()) + return + } + if os.Rename(tmp.Name(), path) != nil { + _ = os.Remove(tmp.Name()) + } +} diff --git a/statelock_unix.go b/statelock_unix.go new file mode 100644 index 0000000..05e213d --- /dev/null +++ b/statelock_unix.go @@ -0,0 +1,36 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +//go:build !windows + +package main + +import ( + "os" + "syscall" +) + +// lockState takes an exclusive advisory lock and returns the release. +// +// flock and not a lockfile created with O_EXCL, because the kernel drops this +// one when the process dies. A cathode that is killed mid-write would otherwise +// leave a lockfile behind that every other instance waits on forever, and +// breaking a stale lock needs a timeout nobody can pick correctly. +// +// The lock is its own file rather than the store, because the store is replaced +// by rename on every write: a lock on that inode would guard a file nobody is +// looking at any more. +func lockState(path string) (func(), error) { + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return nil, err + } + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { + _ = f.Close() + return nil, err + } + return func() { + _ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN) + _ = f.Close() + }, nil +} diff --git a/statelock_windows.go b/statelock_windows.go new file mode 100644 index 0000000..4bfe65b --- /dev/null +++ b/statelock_windows.go @@ -0,0 +1,16 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +//go:build windows + +package main + +// lockState does not lock on Windows, which has no flock. Two instances can then +// interleave a write, and the later one wins. +// +// That is a far smaller window than the bug the lock exists for: the write is a +// read-modify-write either way, so what is at risk is the microseconds between +// the read and the rename, not everything another instance recorded since this +// one started. Reaching for a lockfile instead would trade this for a stale lock +// that wedges every instance, which is worse. +func lockState(string) (func(), error) { return func() {}, nil } From 879cd403dc9e7c17545bf5f01e488c621854b8ec Mon Sep 17 00:00:00 2001 From: tdwd Date: Sat, 19 Sep 2026 19:01:52 +0200 Subject: [PATCH 2/2] state: give settings the same write discipline, and keep tests off the real files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two remaining instances of the shape the previous commit fixed. settings.json had it, and the fix is the interesting one: a read-modify-write alone cannot do it, because re-reading tells you what is on disk but not which of the caller's fields is a deliberate change and which is a stale copy. So saveSettings takes a mutator rather than a settings value — the caller names the field it is changing, and the signature no longer accepts a whole struct. model.commitSetting applies the same mutator to the model and to the file and is the single owner of a settings write; there is no longer a direct m.settings.X assignment anywhere. A theme commits its banner in the same mutator, or the file can end up holding one theme with the other's banner. The running UI deliberately does not re-read: a theme another window picked must not restyle this one mid-session. And the test suite reached the developer's own state dir. Any test that builds a model opens the session store and the prompt history, and most never redirected XDG_STATE_HOME — so `go test` read those files, and because an established prompt history sits at its cap, the cap self-heal rewrote the real prompt-history.jsonl. A package TestMain now points every test at a throwaway dir, which is what makes it unreachable: a new test cannot touch them by forgetting a t.Setenv. replaceFile is now the only way any state file is replaced, so the atomic write cannot be right in one store and wrong in another. It gained a Sync before the rename, which is load-bearing rather than hygiene: without it the rename can reach the disk while the bytes have not, leaving a file that is present and empty — the one crash the temp-and-rename is there to rule out. settings.go was 325 lines, so the menu rows and their commit functions moved to settingsmenu.go, leaving settings.go as the persisted struct and its file. --- compactstyle.go | 3 +- diff_split.go | 3 +- history.go | 6 +- history_test.go | 9 +- main_test.go | 36 ++++++++ settings.go | 228 ++++++---------------------------------------- settings_test.go | 61 ++++++++++++- settingsmenu.go | 211 ++++++++++++++++++++++++++++++++++++++++++ sidebar.go | 3 +- statefile.go | 66 +++++++++----- statelock_unix.go | 7 +- sysprompt.go | 3 +- 12 files changed, 394 insertions(+), 242 deletions(-) create mode 100644 main_test.go create mode 100644 settingsmenu.go diff --git a/compactstyle.go b/compactstyle.go index 7ecdfc3..81e4ec6 100644 --- a/compactstyle.go +++ b/compactstyle.go @@ -64,8 +64,7 @@ func barItems() []pickerItem { // commitBar persists the compact-bar animation. Chrome only — nothing in the // transcript re-renders, so the next frame just draws the new style. func (m *model) commitBar(id string) { - m.settings.Bar = id - saveSettings(m.settings) + m.commitSetting(func(s *settings) { s.Bar = id }) m.add(entInfo, "→ compact bar: "+barLabel(id)) } diff --git a/diff_split.go b/diff_split.go index b8187c8..dfc0094 100644 --- a/diff_split.go +++ b/diff_split.go @@ -48,8 +48,7 @@ func diffItems() []pickerItem { // commitDiff applies the chosen diff style, re-renders the transcript's existing // diff cards in it, and persists the choice. func (m *model) commitDiff(id string) { - m.settings.Diff = id - saveSettings(m.settings) + m.commitSetting(func(s *settings) { s.Diff = id }) m.rerender() m.add(entInfo, "→ diff: "+diffLabel(id)) } diff --git a/history.go b/history.go index 1f98ab1..a542cbe 100644 --- a/history.go +++ b/history.go @@ -26,8 +26,8 @@ type promptEntry struct { // entries is a cache of that file, not the store: every running cathode // instance shares it, so the write rules in historyfile.go are what keep one // window from erasing another's prompts. This comment used to claim appends -// "touch only the tail in the common case", which is what hid the fact that -// past the cap they did not — see trim. +// "touch only the tail in the common case", which is what hid the fact that past +// the cap they did not — see load. type history struct { mu sync.Mutex entries []promptEntry @@ -46,7 +46,7 @@ func openHistory() *history { h := &history{path: path} h.mu.Lock() defer h.mu.Unlock() - h.load() // held-lock, like every other caller + h.load() // held-lock, as load documents return h } diff --git a/history_test.go b/history_test.go index c3da26d..085cf4a 100644 --- a/history_test.go +++ b/history_test.go @@ -38,14 +38,15 @@ func TestHistoryAppendCapsAtMax(t *testing.T) { } // The prompt-history file is shared by every cathode instance, and the cap is -// small enough that an established history trims on nearly every turn. So the -// trim must re-read the file rather than rebuild it from one instance's memory, -// or each window erases the other's prompts as fast as they are typed. +// small enough that an established history reaches the cap self-heal on nearly +// every turn. So load must re-read the file rather than rebuild it from one +// instance's memory, or each window erases the other's prompts as fast as they +// are typed. func TestHistoryKeepsWhatAnotherInstanceAppended(t *testing.T) { path := filepath.Join(t.TempDir(), "prompt-history.jsonl") a, b := &history{path: path}, &history{path: path} - // Fill past the cap so every further append takes the trim path. + // Fill past the cap, so every further append reaches the self-heal. for i := 0; i < maxHistoryEntries; i++ { a.Append(fmt.Sprintf("a-%02d", i)) } diff --git a/main_test.go b/main_test.go new file mode 100644 index 0000000..da6e104 --- /dev/null +++ b/main_test.go @@ -0,0 +1,36 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "os" + "testing" +) + +// TestMain points the whole package at a throwaway state dir. +// +// Any test that builds a model opens the session store and the prompt history, +// and most of them never redirected $XDG_STATE_HOME — so `go test` read the +// developer's own state, and because an established prompt history sits at its +// cap, the cap self-heal rewrote their real prompt-history.jsonl. +// +// Doing it here rather than in each test is what makes that unreachable: a new +// test cannot touch those files by forgetting a t.Setenv. A test that needs a +// specific dir still overrides this with its own t.Setenv, which is scoped to +// that test and restored after it. +func TestMain(m *testing.M) { + dir, err := os.MkdirTemp("", "cathode-test-state-*") + if err != nil { + fmt.Fprintln(os.Stderr, "cannot create a test state dir:", err) + os.Exit(1) + } + if err := os.Setenv("XDG_STATE_HOME", dir); err != nil { + fmt.Fprintln(os.Stderr, "cannot redirect XDG_STATE_HOME:", err) + os.Exit(1) + } + code := m.Run() + _ = os.RemoveAll(dir) // not deferred: os.Exit does not run defers + os.Exit(code) +} diff --git a/settings.go b/settings.go index 235d79f..c54781f 100644 --- a/settings.go +++ b/settings.go @@ -7,203 +7,9 @@ import ( "encoding/json" "os" "path/filepath" - "strconv" ) -// Header animation style ids. These are the values persisted in settings.json -// and the ids dispatched by renderHeader (rainbow.go). -const ( - headerCyan = "cyan" - headerTheme = "theme" - headerRainbow = "rainbow" - headerPulse = "pulse" - headerAmber = "amber" - headerMagenta = "magenta" - headerOff = "off" - - // headerHidden is retired. It used to be a header *animation* value, which - // conflated two different things: how the wordmark moves, and whether the - // banner exists at all. That conflation is why a theme could not own - // banner visibility without also discarding the user's animation choice. - // Visibility now lives in settings.Banner; this const remains only so - // loadSettings can migrate an old settings.json. - headerHidden = "hidden" -) - -// Banner visibility. Separate from the header animation because a theme owns -// whether the banner shows, while the user owns how its wordmark animates. -const ( - bannerOn = "on" - bannerOff = "off" -) - -// bannerDef is one row in the /settings banner picker. -type bannerDef struct{ id, label, desc string } - -var bannerModes = []bannerDef{ - {bannerOn, "shown", "the wordmark banner and its session divider"}, - {bannerOff, "hidden", "no banner or divider — gives the transcript 4 more rows"}, -} - -func bannerLabel(id string) string { - for _, b := range bannerModes { - if b.id == id { - return b.label - } - } - return id -} - -func bannerItems() []pickerItem { - items := make([]pickerItem, 0, len(bannerModes)) - for _, b := range bannerModes { - items = append(items, pickerItem{id: b.id, title: b.label, subtitle: b.desc}) - } - return items -} - -// commitBanner shows or hides the banner and persists it. The banner occupies -// rows, so the viewport has to be resized whenever it appears or disappears. -func (m *model) commitBanner(id string) { - if id == m.settings.Banner { - return - } - m.settings.Banner = id - saveSettings(m.settings) - m.resizeViewport() - m.add(entInfo, "→ banner: "+bannerLabel(id)) -} - -// headerStyleDef is one row in the /settings header picker. -type headerStyleDef struct{ id, label, desc string } - -// headerStyles is the ordered set shown in the settings modal. Add a style by -// adding a case to renderHeader and a row here. -var headerStyles = []headerStyleDef{ - {headerTheme, "theme color", "shimmer in the active theme's primary color (matches the ornaments)"}, - {headerCyan, "cyan shimmer", "single bright-cyan brightness wave (fixed, ignores theme)"}, - {headerRainbow, "rainbow", "the full-spectrum hue cycle"}, - {headerPulse, "cyan pulse", "eases between light and dark cyan"}, - {headerAmber, "amber shimmer", "single amber/gold brightness wave"}, - {headerMagenta, "magenta shimmer", "single magenta brightness wave"}, - {headerOff, "off (static)", "no animation — static accent color"}, -} - -func headerStyleLabel(id string) string { - for _, s := range headerStyles { - if s.id == id { - return s.label - } - } - return id -} - -func headerStyleItems() []pickerItem { - items := make([]pickerItem, 0, len(headerStyles)) - for _, s := range headerStyles { - items = append(items, pickerItem{id: s.id, title: s.label, subtitle: s.desc}) - } - return items -} - -// Animation frame rate (the header wordmark) in fps. Lower = fewer redraws = -// less CPU; the header style "off" stops the animation entirely (zero idle -// redraws). Persisted as settings.FPS. -const defaultFPS = 12 - -type fpsOption struct { - fps int - label, desc string -} - -var fpsOptions = []fpsOption{ - {24, "24 fps", "smoothest header animation — highest CPU"}, - {12, "12 fps", "smooth (default)"}, - {6, "6 fps", "calmer, lower CPU"}, - {3, "3 fps", "minimal CPU while still animating"}, -} - -func fpsLabel(fps int) string { - for _, o := range fpsOptions { - if o.fps == fps { - return o.label - } - } - return strconv.Itoa(fps) + " fps" -} - -func fpsItems() []pickerItem { - items := make([]pickerItem, 0, len(fpsOptions)) - for _, o := range fpsOptions { - items = append(items, pickerItem{id: strconv.Itoa(o.fps), title: o.label, subtitle: o.desc}) - } - return items -} - -// commitFPS applies the chosen animation rate and persists it. (For zero idle -// redraws, set the header animation itself to "off".) -func (m *model) commitFPS(id string) { - fps, err := strconv.Atoi(id) - if err != nil || fps <= 0 { - return - } - m.settings.FPS = fps - saveSettings(m.settings) - m.add(entInfo, "→ animation: "+fpsLabel(fps)) -} - -// settingsItems is the top-level /settings menu: one row per setting, each -// showing its current value. Selecting a row opens that setting's picker. -func (m *model) settingsItems() []pickerItem { - return []pickerItem{ - {id: "header", title: "header animation", subtitle: "current: " + headerStyleLabel(m.settings.Header)}, - {id: "banner", title: "banner", subtitle: "current: " + bannerLabel(m.settings.Banner) + " · the color theme sets this"}, - {id: "fps", title: "animation fps", subtitle: "current: " + fpsLabel(m.settings.FPS) + " · lower = less CPU"}, - {id: "theme", title: "color theme", subtitle: "current: " + themeLabel(m.settings.Theme)}, - {id: "diff", title: "diff style", subtitle: "current: " + diffLabel(m.settings.Diff)}, - {id: "sidebarpos", title: "sidebar position", subtitle: "current: " + sidebarLabel(m.settings.Sidebar)}, - {id: "bar", title: "compact bar", subtitle: "current: " + barLabel(m.settings.Bar) + " · the /compact progress animation"}, - {id: "sysprompt", title: "extra system prompt", subtitle: "current: " + sysPromptLabel(m.settings.SysPrompt) + sysPromptEditedNote(m.sysPromptEdited()) + " · restarts the session to apply"}, - } -} - -// commitHeaderStyle applies the chosen header animation live and persists it. -// Called when the user presses Enter in the /settings header picker. -// -// Animation never changes the frame's height — that is settings.Banner's job — -// so this is a repaint and needs no resize. -func (m *model) commitHeaderStyle(id string) { - m.headerStyle = id - m.settings.Header = id - saveSettings(m.settings) - m.add(entInfo, "→ header: "+headerStyleLabel(id)) -} - -// commitTheme applies the chosen color theme live (rebuilding every style) and -// persists it. rebuild() refreshes the transcript's themed parts; the chrome -// repaints on the next frame. -func (m *model) commitTheme(id string) { - applyTheme(id) - m.settings.Theme = id - - // The theme owns whether the banner shows — see themeDef.banner. Every - // theme states it, so switching toggles in both directions: away from a - // bannerless theme brings the banner back. An earlier version let a theme - // stay silent, which meant switching away from cinder left it hidden. - // - // It is announced rather than applied silently, because it changes the - // frame's height and the user asked for a palette, not a resize. - note := "" - if want := bannerFor(id); want != m.settings.Banner { - m.settings.Banner = want - m.resizeViewport() - note = " · banner: " + bannerLabel(want) - } - - saveSettings(m.settings) - m.rerender() // re-render the whole transcript in the new palette - m.add(entInfo, "→ theme: "+themeLabel(id)+note) -} +// ---- the persisted settings file ---- // settings is the persisted user config. Small and forward-compatible: unknown // fields are ignored on load, missing ones take their default. @@ -283,9 +89,17 @@ func loadSettings() settings { return s } -// saveSettings writes settings.json best-effort; failures are silent (settings -// are a nicety, not load-bearing). -func saveSettings(s settings) { +// saveSettings applies fn to the settings on disk and writes them back, +// best-effort; failures are silent (settings are a nicety, not load-bearing). +// +// It takes a mutator and not a settings value, and that is the whole point. +// Every cathode instance shares this file and each holds a copy loaded at its +// own start, so writing that copy back reverted every setting another window had +// changed since — a theme switch in one window undid a diff-style switch in +// another. A read-modify-write alone cannot fix it, because it cannot tell a +// stale field from a deliberate one. So the caller names the field it is +// changing, and the signature no longer accepts a whole stale struct. +func saveSettings(fn func(*settings)) { p := settingsPath() if p == "" { return @@ -293,7 +107,23 @@ func saveSettings(s settings) { if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { return } + if unlock, err := lockState(p + ".lock"); err == nil { + defer unlock() + } + s := loadSettings() // re-read inside the lock, like every other state file + fn(&s) if b, err := json.MarshalIndent(s, "", " "); err == nil { - _ = os.WriteFile(p, b, 0o644) + replaceFile(p, b) } } + +// commitSetting changes one setting in this model and on disk, so the two cannot +// disagree about what was changed. Every commit* function goes through it. +// +// The model's own copy is not re-read from the file on purpose: a setting another +// window changed must not alter this window's theme mid-session. The file merges; +// the running UI does not. +func (m *model) commitSetting(fn func(*settings)) { + fn(&m.settings) + saveSettings(fn) +} diff --git a/settings_test.go b/settings_test.go index 1b3064a..5aa2ce5 100644 --- a/settings_test.go +++ b/settings_test.go @@ -19,17 +19,21 @@ func TestSettingsRoundTrip(t *testing.T) { t.Fatalf("default Header = %q, want %q", got.Header, headerCyan) } - saveSettings(settings{Header: headerRainbow}) + saveSettings(func(s *settings) { s.Header = headerRainbow }) if got := loadSettings(); got.Header != headerRainbow { t.Fatalf("after save, Header = %q, want %q", got.Header, headerRainbow) } - if _, err := os.Stat(filepath.Join(dir, "cathode", "settings.json")); err != nil { + path := filepath.Join(dir, "cathode", "settings.json") + if _, err := os.Stat(path); err != nil { t.Fatalf("settings.json not written: %v", err) } - // An all-empty file is also what a settings.json written before a field - // existed looks like — every field must fall back, not land on "". - saveSettings(settings{Header: ""}) + // An all-empty file is what a settings.json written before a field existed + // looks like — every field must fall back, not land on "". Written directly, + // because saveSettings now merges into the defaults and so cannot produce one. + if err := os.WriteFile(path, []byte(`{"header":""}`), 0o644); err != nil { + t.Fatal(err) + } got := loadSettings() if got.Header != headerCyan { t.Fatalf("empty Header should fall back to %q, got %q", headerCyan, got.Header) @@ -39,6 +43,53 @@ func TestSettingsRoundTrip(t *testing.T) { } } +// A theme carries whether the banner shows, so both must land in the file +// together. Written one at a time, the file can hold this theme with the previous +// one's banner — and the next launch draws chrome the theme does not want. +func TestCommittingAThemePersistsItsBanner(t *testing.T) { + t.Setenv("XDG_STATE_HOME", t.TempDir()) + m := newModel(launchConfig{Engine: &fakeEngine{}, Mode: "ask", Spinner: "bar"}) + + m.commitTheme("cinder") // the one bannerless theme + if got := loadSettings(); got.Theme != "cinder" || got.Banner != bannerOff { + t.Errorf("stored (%q, %q), want (cinder, %q)", got.Theme, got.Banner, bannerOff) + } + + // And back: every other theme states that it wants the banner, so switching + // away has to bring it back rather than leaving it hidden. + m.commitTheme(defaultTheme) + if got := loadSettings(); got.Theme != defaultTheme || got.Banner != bannerOn { + t.Errorf("stored (%q, %q), want (%q, %q)", got.Theme, got.Banner, defaultTheme, bannerOn) + } +} + +// A setting changed in one window must not revert one changed in another. Each +// window holds a copy loaded at its own start, so writing that copy back was +// what reverted the other — the bug sessionStore documents, in settings.json. +func TestSavingOneSettingKeepsAnotherWindowsChange(t *testing.T) { + t.Setenv("XDG_STATE_HOME", t.TempDir()) + a := newModel(launchConfig{Engine: &fakeEngine{}, Mode: "ask", Spinner: "bar"}) + b := newModel(launchConfig{Engine: &fakeEngine{}, Mode: "ask", Spinner: "bar"}) + + a.commitSetting(func(s *settings) { s.Diff = diffSplit }) + // B loaded before that and still holds the default diff style. Changing its + // theme must write the theme, not B's whole copy. + b.commitSetting(func(s *settings) { s.Theme = "cinder" }) + + got := loadSettings() + if got.Diff != diffSplit { + t.Errorf("Diff = %q, want %q kept across the other window's write", got.Diff, diffSplit) + } + if got.Theme != "cinder" { + t.Errorf("Theme = %q, want cinder", got.Theme) + } + // And B keeps its own view: a setting changed elsewhere must not restyle a + // running window mid-session. + if b.settings.Diff == diffSplit { + t.Error("B's in-memory diff style followed the file; a running window keeps its own") + } +} + // TestRenderHeaderEveryStyle ensures every row in the /settings picker maps to a // renderHeader branch that produces output (no orphaned ids). func TestRenderHeaderEveryStyle(t *testing.T) { diff --git a/settingsmenu.go b/settingsmenu.go new file mode 100644 index 0000000..a8d2c58 --- /dev/null +++ b/settingsmenu.go @@ -0,0 +1,211 @@ +// Copyright 2026 Triple Down AB +// SPDX-License-Identifier: Apache-2.0 + +package main + +import "strconv" + +// ---- the /settings menu and what each row commits ---- +// +// Every commit* function here changes one setting through model.commitSetting +// (settings.go), which owns the write. + +// Header animation style ids. These are the values persisted in settings.json +// and the ids dispatched by renderHeader (rainbow.go). +const ( + headerCyan = "cyan" + headerTheme = "theme" + headerRainbow = "rainbow" + headerPulse = "pulse" + headerAmber = "amber" + headerMagenta = "magenta" + headerOff = "off" + + // headerHidden is retired. It used to be a header *animation* value, which + // conflated two different things: how the wordmark moves, and whether the + // banner exists at all. That conflation is why a theme could not own + // banner visibility without also discarding the user's animation choice. + // Visibility now lives in settings.Banner; this const remains only so + // loadSettings can migrate an old settings.json. + headerHidden = "hidden" +) + +// Banner visibility. Separate from the header animation because a theme owns +// whether the banner shows, while the user owns how its wordmark animates. +const ( + bannerOn = "on" + bannerOff = "off" +) + +// bannerDef is one row in the /settings banner picker. +type bannerDef struct{ id, label, desc string } + +var bannerModes = []bannerDef{ + {bannerOn, "shown", "the wordmark banner and its session divider"}, + {bannerOff, "hidden", "no banner or divider — gives the transcript 4 more rows"}, +} + +func bannerLabel(id string) string { + for _, b := range bannerModes { + if b.id == id { + return b.label + } + } + return id +} + +func bannerItems() []pickerItem { + items := make([]pickerItem, 0, len(bannerModes)) + for _, b := range bannerModes { + items = append(items, pickerItem{id: b.id, title: b.label, subtitle: b.desc}) + } + return items +} + +// commitBanner shows or hides the banner and persists it. The banner occupies +// rows, so the viewport has to be resized whenever it appears or disappears. +func (m *model) commitBanner(id string) { + if id == m.settings.Banner { + return + } + m.commitSetting(func(s *settings) { s.Banner = id }) + m.resizeViewport() + m.add(entInfo, "→ banner: "+bannerLabel(id)) +} + +// headerStyleDef is one row in the /settings header picker. +type headerStyleDef struct{ id, label, desc string } + +// headerStyles is the ordered set shown in the settings modal. Add a style by +// adding a case to renderHeader and a row here. +var headerStyles = []headerStyleDef{ + {headerTheme, "theme color", "shimmer in the active theme's primary color (matches the ornaments)"}, + {headerCyan, "cyan shimmer", "single bright-cyan brightness wave (fixed, ignores theme)"}, + {headerRainbow, "rainbow", "the full-spectrum hue cycle"}, + {headerPulse, "cyan pulse", "eases between light and dark cyan"}, + {headerAmber, "amber shimmer", "single amber/gold brightness wave"}, + {headerMagenta, "magenta shimmer", "single magenta brightness wave"}, + {headerOff, "off (static)", "no animation — static accent color"}, +} + +func headerStyleLabel(id string) string { + for _, s := range headerStyles { + if s.id == id { + return s.label + } + } + return id +} + +func headerStyleItems() []pickerItem { + items := make([]pickerItem, 0, len(headerStyles)) + for _, s := range headerStyles { + items = append(items, pickerItem{id: s.id, title: s.label, subtitle: s.desc}) + } + return items +} + +// Animation frame rate (the header wordmark) in fps. Lower = fewer redraws = +// less CPU; the header style "off" stops the animation entirely (zero idle +// redraws). Persisted as settings.FPS. +const defaultFPS = 12 + +type fpsOption struct { + fps int + label, desc string +} + +var fpsOptions = []fpsOption{ + {24, "24 fps", "smoothest header animation — highest CPU"}, + {12, "12 fps", "smooth (default)"}, + {6, "6 fps", "calmer, lower CPU"}, + {3, "3 fps", "minimal CPU while still animating"}, +} + +func fpsLabel(fps int) string { + for _, o := range fpsOptions { + if o.fps == fps { + return o.label + } + } + return strconv.Itoa(fps) + " fps" +} + +func fpsItems() []pickerItem { + items := make([]pickerItem, 0, len(fpsOptions)) + for _, o := range fpsOptions { + items = append(items, pickerItem{id: strconv.Itoa(o.fps), title: o.label, subtitle: o.desc}) + } + return items +} + +// commitFPS applies the chosen animation rate and persists it. (For zero idle +// redraws, set the header animation itself to "off".) +func (m *model) commitFPS(id string) { + fps, err := strconv.Atoi(id) + if err != nil || fps <= 0 { + return + } + m.commitSetting(func(s *settings) { s.FPS = fps }) + m.add(entInfo, "→ animation: "+fpsLabel(fps)) +} + +// settingsItems is the top-level /settings menu: one row per setting, each +// showing its current value. Selecting a row opens that setting's picker. +func (m *model) settingsItems() []pickerItem { + return []pickerItem{ + {id: "header", title: "header animation", subtitle: "current: " + headerStyleLabel(m.settings.Header)}, + {id: "banner", title: "banner", subtitle: "current: " + bannerLabel(m.settings.Banner) + " · the color theme sets this"}, + {id: "fps", title: "animation fps", subtitle: "current: " + fpsLabel(m.settings.FPS) + " · lower = less CPU"}, + {id: "theme", title: "color theme", subtitle: "current: " + themeLabel(m.settings.Theme)}, + {id: "diff", title: "diff style", subtitle: "current: " + diffLabel(m.settings.Diff)}, + {id: "sidebarpos", title: "sidebar position", subtitle: "current: " + sidebarLabel(m.settings.Sidebar)}, + {id: "bar", title: "compact bar", subtitle: "current: " + barLabel(m.settings.Bar) + " · the /compact progress animation"}, + {id: "sysprompt", title: "extra system prompt", subtitle: "current: " + sysPromptLabel(m.settings.SysPrompt) + sysPromptEditedNote(m.sysPromptEdited()) + " · restarts the session to apply"}, + } +} + +// commitHeaderStyle applies the chosen header animation live and persists it. +// Called when the user presses Enter in the /settings header picker. +// +// Animation never changes the frame's height — that is settings.Banner's job — +// so this is a repaint and needs no resize. +func (m *model) commitHeaderStyle(id string) { + m.headerStyle = id + m.commitSetting(func(s *settings) { s.Header = id }) + m.add(entInfo, "→ header: "+headerStyleLabel(id)) +} + +// commitTheme applies the chosen color theme live (rebuilding every style) and +// persists it. rebuild() refreshes the transcript's themed parts; the chrome +// repaints on the next frame. +func (m *model) commitTheme(id string) { + applyTheme(id) + + // The theme owns whether the banner shows — see themeDef.banner. Every + // theme states it, so switching toggles in both directions: away from a + // bannerless theme brings the banner back. An earlier version let a theme + // stay silent, which meant switching away from cinder left it hidden. + // + // It is announced rather than applied silently, because it changes the + // frame's height and the user asked for a palette, not a resize. + // Read before the commit, because commitSetting is what changes m.settings. + banner := bannerFor(id) + shown := banner != m.settings.Banner + + // Both fields in one mutator: a theme carries its banner setting, so writing + // them one at a time could leave the file holding this theme with the + // previous one's banner. + m.commitSetting(func(s *settings) { + s.Theme = id + s.Banner = banner + }) + + note := "" + if shown { + m.resizeViewport() + note = " · banner: " + bannerLabel(banner) + } + m.rerender() // re-render the whole transcript in the new palette + m.add(entInfo, "→ theme: "+themeLabel(id)+note) +} diff --git a/sidebar.go b/sidebar.go index e24e111..713d646 100644 --- a/sidebar.go +++ b/sidebar.go @@ -49,8 +49,7 @@ func sidebarPosItems() []pickerItem { // commitSidebarPos persists the side the info rail sits on. The body memo keys // on it (see bodyKey), so the next frame repositions the rail. func (m *model) commitSidebarPos(id string) { - m.settings.Sidebar = id - saveSettings(m.settings) + m.commitSetting(func(s *settings) { s.Sidebar = id }) m.add(entInfo, "→ sidebar: "+sidebarLabel(id)) } diff --git a/statefile.go b/statefile.go index 76b4e94..ef1ce51 100644 --- a/statefile.go +++ b/statefile.go @@ -10,14 +10,18 @@ import ( "path/filepath" ) -// ---- reading and replacing a JSONL state file ---- +// ---- reading and replacing a state file ---- // -// Both persisted stores are JSONL under $XDG_STATE_HOME/cathode, and both are -// shared by every running cathode instance. The two operations that has to get -// right live here rather than once per store, because a copy differing by one -// detail is how a shared file loses data quietly: the session store and the -// prompt history each grew their own, and only one of them ended up with a +// Every file under $XDG_STATE_HOME/cathode is shared by every running cathode +// instance, so how one is read and how one is replaced both have to be right. +// They live here rather than once per store, because a copy differing by a +// single detail is how a shared file loses data quietly: the session store and +// the prompt history each grew their own, and only one of them ended up with a // unique temp name. +// +// readJSONL and writeJSONL serve the two JSONL stores. replaceFile is the +// atomic write under both, and settings.json (a single JSON object) uses it +// directly. // maxStateLine bounds one record. A pasted prompt is the longest thing either // store holds, and a longer line is dropped rather than growing the scanner @@ -45,33 +49,51 @@ func readJSONL[T any](path string) []T { return out } -// writeJSONL replaces path with one JSON line per record. -// -// Atomic via a temp file and a rename, so a crash cannot leave the file -// half-truncated and a concurrent reader sees either the old file or the new -// one. The temp name is unique: two instances sharing one ".tmp" write -// into the same file and rename a half-finished one into place, which is the one -// way a whole store goes at once. +// writeJSONL replaces path with one JSON line per record. A record that will not +// marshal is left out rather than failing the write, so one bad row costs its own +// line and not the file. // // Callers must hold the file lock (lockState) and must have re-read the file // inside it. Replacing a file that several instances write means starting from // what is on disk, not from a cache loaded at process start — see sessionStore // for what that cost. func writeJSONL[T any](path string, records []T) { - tmp, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".tmp-*") - if err != nil { - return - } + var body []byte for _, r := range records { b, err := json.Marshal(r) if err != nil { continue } - if _, err := tmp.Write(append(b, '\n')); err != nil { - _ = tmp.Close() - _ = os.Remove(tmp.Name()) - return - } + body = append(append(body, b...), '\n') + } + replaceFile(path, body) +} + +// replaceFile writes body to path atomically: a temp file and a rename, so a +// crash cannot leave the file half-truncated and a concurrent reader sees either +// the old file or the new one. +// +// The temp name is unique. Two instances sharing one ".tmp" write into the +// same file and rename a half-finished one into place, which is the one way a +// whole state file goes at once. This is the only way any state file is +// replaced, so that cannot be got right in one store and wrong in another. +func replaceFile(path string, body []byte) { + tmp, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".tmp-*") + if err != nil { + return + } + if _, err := tmp.Write(body); err != nil { + _ = tmp.Close() + _ = os.Remove(tmp.Name()) + return + } + // Sync before the rename, or the rename can reach the disk while the bytes + // have not — which leaves a file that is present and empty. That is the one + // crash this is supposed to rule out, so the claim above needs this line. + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + _ = os.Remove(tmp.Name()) + return } if err := tmp.Close(); err != nil { _ = os.Remove(tmp.Name()) diff --git a/statelock_unix.go b/statelock_unix.go index 05e213d..f9098a8 100644 --- a/statelock_unix.go +++ b/statelock_unix.go @@ -19,7 +19,12 @@ import ( // // The lock is its own file rather than the store, because the store is replaced // by rename on every write: a lock on that inode would guard a file nobody is -// looking at any more. +// looking at any more. The lockfile itself is never replaced, so it stays in the +// state dir between runs. +// +// The wait is unbounded, which is right when every holder keeps it for one small +// read and write. If a cathode is ever seen frozen on a keypress, a sibling +// stopped mid-write is the thing to look for. func lockState(path string) (func(), error) { f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) if err != nil { diff --git a/sysprompt.go b/sysprompt.go index 323f800..89c77cc 100644 --- a/sysprompt.go +++ b/sysprompt.go @@ -191,8 +191,7 @@ func (m *model) commitSysPrompt(id string) tea.Cmd { m.add(entError, "no system prompt to apply — write one to "+sysPromptPath()+" first") return nil } - m.settings.SysPrompt = on - saveSettings(m.settings) + m.commitSetting(func(s *settings) { s.SysPrompt = on }) what := "extra system prompt: " + sysPromptLabel(on) if edited {