From 7387f7584fbd620cb8a180eb6042f1452906322b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20F=C3=A1bi=C3=A1n?= Date: Sun, 5 Jul 2026 10:26:58 +0200 Subject: [PATCH 1/3] fix(ui): persist group create/rename/move even during a reload window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a root group with 'g' or renaming one with 'r' only worked every second try. The GroupDialog Create/Rename/Move handlers persisted via saveInstances(), which is a no-op while isReloading (and aborts on the mtime external-change check). Each successful group save triggers a storage-watcher reload; if the next group mutation lands in that window its save is skipped, and when the reload rebuilds groupTree from disk the mutation vanishes — the alternating 'every second try' behavior. Session creation already uses forceSaveInstances() for exactly this reason ('was losing groups!'), and session renames get pendingTitleChanges replay; group mutations had neither. Switch the three group-structure mutations to forceSaveInstances() so they persist regardless of the reload flag. Regression tests drive the real handler with isReloading=true and assert the group survives a reload round-trip through real SQLite storage. --- .../ui/group_persist_during_reload_test.go | 108 ++++++++++++++++++ internal/ui/home.go | 13 ++- 2 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 internal/ui/group_persist_during_reload_test.go diff --git a/internal/ui/group_persist_during_reload_test.go b/internal/ui/group_persist_during_reload_test.go new file mode 100644 index 00000000..b73aa431 --- /dev/null +++ b/internal/ui/group_persist_during_reload_test.go @@ -0,0 +1,108 @@ +package ui + +import ( + "os" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/asheshgoplani/agent-deck/internal/session" +) + +// These tests are the regression guard for the "new main group only works +// every second try" / "rename group only works every second try" bug. +// +// Root cause: GroupDialogCreate/Rename persisted via saveInstances(), which is +// SKIPPED while isReloading (and aborts on the mtime external-change check). A +// group mutation that landed during a storage-watcher reload window was written +// to memory but never to disk, so when the reload rebuilt groupTree from disk +// the mutation vanished — hence the alternating "every second try". The fix +// switches these paths to forceSaveInstances(), which persists regardless of +// the reload flag (mirroring session creation's "was losing groups!" forceSave). + +// testHomeWithStorage builds a minimally-wired Home backed by real SQLite +// storage under an isolated temp HOME, with one seed instance so the +// empty-overwrite guard in saveInstancesWithForce does not refuse the write. +func testHomeWithStorage(t *testing.T, profile string) (*Home, *session.Storage) { + t.Helper() + origHome := os.Getenv("HOME") + os.Setenv("HOME", t.TempDir()) + session.ClearUserConfigCache() + t.Cleanup(func() { + os.Setenv("HOME", origHome) + session.ClearUserConfigCache() + }) + + storage, err := session.NewStorageWithProfile(profile) + if err != nil { + t.Fatalf("NewStorageWithProfile: %v", err) + } + t.Cleanup(func() { storage.Close() }) + + seed := session.NewInstance("seed", "/tmp/seed") + h := &Home{ + storage: storage, + profile: profile, + instances: []*session.Instance{seed}, + groupTree: session.NewGroupTree([]*session.Instance{seed}), + groupDialog: NewGroupDialog(), + } + return h, storage +} + +func groupPersisted(t *testing.T, storage *session.Storage, name string) bool { + t.Helper() + _, groups, err := storage.LoadWithGroups() + if err != nil { + t.Fatalf("LoadWithGroups: %v", err) + } + for _, g := range groups { + if g.Name == name { + return true + } + } + return false +} + +func TestGroupCreatePersistsDuringReload(t *testing.T) { + h, storage := testHomeWithStorage(t, "_group_persist_create") + + // Simulate a storage-watcher reload in flight — the window in which the old + // saveInstances() silently dropped the write. + h.isReloading = true + + // Drive the real create flow: create-mode dialog + Enter. + h.groupDialog.Show() + h.groupDialog.nameInput.SetValue("alpha") + h.handleGroupDialogKey(tea.KeyMsg{Type: tea.KeyEnter}) + + if !groupPersisted(t, storage, "alpha") { + t.Error(`group "alpha" created during a reload window was not persisted; ` + + `create must use forceSaveInstances (saveInstances is skipped while isReloading)`) + } +} + +func TestGroupRenamePersistsDuringReload(t *testing.T) { + h, storage := testHomeWithStorage(t, "_group_persist_rename") + + // Seed and persist a group while NOT reloading. + h.groupTree.CreateGroup("old") + h.forceSaveInstances() + if !groupPersisted(t, storage, "old") { + t.Fatal("precondition: seed group 'old' should be persisted") + } + + // Now rename it during a reload window. + h.isReloading = true + h.groupDialog.ShowRename("old", "old") + h.groupDialog.nameInput.SetValue("new") + h.handleGroupDialogKey(tea.KeyMsg{Type: tea.KeyEnter}) + + if groupPersisted(t, storage, "old") { + t.Error(`old group name still present after rename during reload window`) + } + if !groupPersisted(t, storage, "new") { + t.Error(`renamed group "new" was not persisted during reload window; ` + + `rename must use forceSaveInstances`) + } +} diff --git a/internal/ui/home.go b/internal/ui/home.go index 02141191..f7b9b5b2 100644 --- a/internal/ui/home.go +++ b/internal/ui/home.go @@ -9619,7 +9619,14 @@ func (h *Home) handleGroupDialogKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } } h.rebuildFlatItems() - h.saveInstances() // Persist the new group + // forceSave, not saveInstances: a save is SKIPPED while + // isReloading (and aborts on the mtime external-change check), + // so a group created during a storage-watcher reload window is + // dropped when the reload rebuilds groupTree from disk — the + // "new group only works every second try" bug. Group mutations + // are user-initiated and MUST persist, like session creation + // (see the "was losing groups!" forceSave in sessionCreatedMsg). + h.forceSaveInstances() // Persist the new group } case GroupDialogRename: name := h.groupDialog.GetValue() @@ -9629,7 +9636,7 @@ func (h *Home) handleGroupDialogKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { h.instances = h.groupTree.GetAllInstances() h.instancesMu.Unlock() h.rebuildFlatItems() - h.saveInstances() + h.forceSaveInstances() // MUST persist even during reload (see GroupDialogCreate) } case GroupDialogMove: targetGroupPath := h.groupDialog.GetSelectedGroup() @@ -9641,7 +9648,7 @@ func (h *Home) handleGroupDialogKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { h.instances = h.groupTree.GetAllInstances() h.instancesMu.Unlock() h.rebuildFlatItems() - h.saveInstances() + h.forceSaveInstances() // MUST persist even during reload (see GroupDialogCreate) } } case GroupDialogRenameSession: From 0e4caab8432168d71edfc5281aef3f7df5be6ccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20F=C3=A1bi=C3=A1n?= Date: Mon, 6 Jul 2026 16:48:14 +0200 Subject: [PATCH 2/3] docs(changelog): group create/rename/move persist during reload window (#1539) --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3896e488..3e0f8ab7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Creating, renaming, or moving a group no longer needs two attempts.** The `GroupDialog` create/rename/move handlers persisted via `saveInstances()`, which is skipped while a storage-watcher reload is in flight, so a group mutation landing in that window was silently dropped when the reload rebuilt the group tree from disk — the "create group has to be done twice" bug. These user-initiated group-structure mutations now use `forceSaveInstances()`, matching session creation. ([#1573](https://github.com/asheshgoplani/agent-deck/pull/1573)) (fixes [#1539](https://github.com/asheshgoplani/agent-deck/issues/1539)) + ## [1.10.9] - 2026-07-02 ### Fixed From 15300afd43882d2fde84965cfbc01b10a0ee9d3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20F=C3=A1bi=C3=A1n?= Date: Mon, 6 Jul 2026 17:10:39 +0200 Subject: [PATCH 3/3] test(ui): cover GroupDialogMove persistence during a reload window The fix switched all three group mutations to forceSaveInstances(), but only create and rename had reload-window regression tests (CodeRabbit nit on #1573). Add TestGroupMovePersistsDuringReload: move the seed session into a destination group with isReloading=true and assert the session's GroupPath survives a storage round-trip. Verified it fails when the Move handler is reverted to saveInstances(). --- .../ui/group_persist_during_reload_test.go | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/internal/ui/group_persist_during_reload_test.go b/internal/ui/group_persist_during_reload_test.go index b73aa431..c67d8d2f 100644 --- a/internal/ui/group_persist_during_reload_test.go +++ b/internal/ui/group_persist_during_reload_test.go @@ -64,6 +64,24 @@ func groupPersisted(t *testing.T, storage *session.Storage, name string) bool { return false } +// sessionGroupPath reads the persisted GroupPath of a session straight from +// storage, so a move that only mutated in-memory state (but was skipped by the +// old saveInstances during a reload) is detectable. +func sessionGroupPath(t *testing.T, storage *session.Storage, sessionID string) string { + t.Helper() + instances, _, err := storage.LoadWithGroups() + if err != nil { + t.Fatalf("LoadWithGroups: %v", err) + } + for _, inst := range instances { + if inst.ID == sessionID { + return inst.GroupPath + } + } + t.Fatalf("session %q not found in storage", sessionID) + return "" +} + func TestGroupCreatePersistsDuringReload(t *testing.T) { h, storage := testHomeWithStorage(t, "_group_persist_create") @@ -106,3 +124,43 @@ func TestGroupRenamePersistsDuringReload(t *testing.T) { `rename must use forceSaveInstances`) } } + +func TestGroupMovePersistsDuringReload(t *testing.T) { + h, storage := testHomeWithStorage(t, "_group_persist_move") + + // Seed a destination group and persist the starting state while NOT reloading. + h.groupTree.CreateGroup("dest") + h.forceSaveInstances() + if !groupPersisted(t, storage, "dest") { + t.Fatal("precondition: destination group 'dest' should be persisted") + } + + // The move handler acts on the session under the cursor, so build the flat + // view and park the cursor on the seed session. + seedID := h.instances[0].ID + h.rebuildFlatItems() + found := false + for i, item := range h.flatItems { + if item.Type == session.ItemTypeSession && item.Session != nil && item.Session.ID == seedID { + h.cursor = i + found = true + break + } + } + if !found { + t.Fatal("precondition: seed session not present in flat items") + } + if got := sessionGroupPath(t, storage, seedID); got == "dest" { + t.Fatalf("precondition: seed session already in 'dest' (got %q)", got) + } + + // Move the session into 'dest' during a reload window. + h.isReloading = true + h.groupDialog.ShowMove([]string{"dest"}) + h.handleGroupDialogKey(tea.KeyMsg{Type: tea.KeyEnter}) + + if got := sessionGroupPath(t, storage, seedID); got != "dest" { + t.Errorf("session GroupPath = %q, want %q; move during a reload window "+ + "must use forceSaveInstances (saveInstances is skipped while isReloading)", got, "dest") + } +}