diff --git a/README.md b/README.md index ac3d29b..0e3e42e 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,10 @@ the arrows keep working inside `claude`. | `?` | help | | `q` | quit | +`x` and `c` act on the selected session, so they need the sessions column to +have focus — `tab` moves it there, and the focused column carries the accent +border. `↵` opens from either column. + | Command — press `ctrl+g` first | | |---|---| | `^g d` | dashboard | diff --git a/docs/backlog.md b/docs/backlog.md index 5566d1e..722ab33 100644 --- a/docs/backlog.md +++ b/docs/backlog.md @@ -4,7 +4,7 @@ Work that is decided but not done, and the reasoning behind each. Things deliberately *not* built are in `docs/architecture.md` under "Not built yet"; this file is only for work that should happen. -Last reviewed 2026-08-28. +Last reviewed 2026-09-07. ## ~~1. Exact status from Claude Code hooks~~ — done 2026-08-23 @@ -236,6 +236,65 @@ and three decisions around it. Numbered 13 rather than reusing 12: that number already names the public-tree notice, and a closed entry should not change meaning. +## 14. Deleting a session, and the worktree it leaves + +Closing forgets the record and keeps the worktree, which is the right default +and currently the only one. Nothing in Deck removes what it keeps, so thirty +closed **isolated** sessions are thirty trees under +`$XDG_STATE_HOME/deck/worktrees`, thirty `session/*` branches, and thirty +entries in the project's `git worktree list`. The only way out today is git by +hand. A session that ran in the project directory leaves nothing behind, which +is the distinction the fourth decision below turns on. + +`x` opens a modal with both outcomes rather than growing a second key. +**Close** keeps the worktree and is the default. **Delete** removes the worktree +and the branch. The Delete row carries `gitx.Diff` against `BaseRef` — "3 files +changed, 41 insertions" or "no files changed" — because a confirmation that +states a fact is answerable and one that states a warning is not. Deck already +measures exactly this for `analyse`, so the figure costs nothing new. + +Four decisions the code has to keep. + +**The disk work comes first, and the record is forgotten only if it succeeded.** +A dropped record over a surviving worktree is an orphan the user can no longer +see, retry or name. The order is: stop the runner, unregister from the +coordinator, remove the worktree, delete the branch, then `RemoveSession` and +`Save`. + +**Nothing is forced.** `git worktree remove` refuses a dirty tree and `git +branch -d` refuses unmerged commits. Both refusals are the answer, reported with +the path. A "delete anyway" row is the one affordance the dirty case cannot +afford, and the way out — commit it, or remove it by hand — fits in the notice. +Add force only if that refusal proves to be a real obstacle. + +**The worktree is the gate, the branch is best effort.** A clean worktree whose +branch holds unmerged commits removes fine, and then `branch -d` refuses. The +session is gone and the work is not, which is the right outcome, so the notice +says the branch was kept and why. + +**A non-isolated session has no worktree and no branch.** Its `Dir` is the +project directory itself. It skips the modal and closes as it does today, and +the delete path must never be reachable with one. `coord.workOf` already refuses +one for the neighbouring reason — a shared project directory holds everyone's +edits at once, so its changes cannot be told apart — and the same fact rules out +deleting anything on such a session's behalf. + +The guard that came out of reading the handler shipped ahead of this, because it +was small and needed nothing from the modal. `x` fired whatever column had +focus, while `dashboardSession` resolves through `listIx`, so pressing it on the +projects list closed that project's newest session — a row the keyboard was not +driving. The row was never invisible: `cursorMarker` keeps a dimmed cursor on +the unfocused column deliberately. What was missing was the focus, and +`sectionLeft` already scoped ←/→ by it for the stated reason — the focused +column is drawn with an accent border, and a key that reaches across makes that +border a lie. `focusedSession` is where the rule lives now, and `c` goes through +it too. It matters to this entry because the modal must open on the session the +user pointed at, not on whichever one `listIx` happens to hold. + +Deliberately not this: `x` on the projects column meaning "remove project". What +happens to that project's sessions and their worktrees deserves its own answer, +not one reached in passing inside a session delete. + ## ~~12. A public-repo notice a cloner will meet~~ — done 2026-08-28 Shipped as **This tree is public** in `docs/architecture.md`, placed before diff --git a/internal/ui/app_test.go b/internal/ui/app_test.go index d0186fd..d648c70 100644 --- a/internal/ui/app_test.go +++ b/internal/ui/app_test.go @@ -1,6 +1,7 @@ package ui import ( + "strings" "testing" "github.com/tripledownab/deck/internal/store" @@ -155,3 +156,94 @@ func TestSectionKeysNeverMoveFocus(t *testing.T) { } } } + +// TestCloseIgnoresProjectsColumn is the regression for x closing a session +// nobody had pointed at. +// +// It is TestSectionKeysRespectFocus applied to the one key that cannot be +// undone. The projects list draws no session cursor and the footer does not +// offer x there, so the key was destructive and undocumented in the same place. +// +// Driven through dashboardKey rather than through the helper: the guard being +// correct proves nothing if the route does not reach it. +func TestCloseIgnoresProjectsColumn(t *testing.T) { + t.Setenv("XDG_STATE_HOME", t.TempDir()) + m := modelWith(2) + if m.focus != colProjects { + t.Fatalf("focus = %v, want the projects column", m.focus) + } + + refused, _ := m.dashboardKey(typed("x")) + after := refused.(Model) + if n := len(after.state.Sessions); n != 2 { + t.Errorf("sessions = %d after x on the projects list, want both kept", n) + } + if after.notice == "" { + t.Error("the refused key said nothing about why") + } + + // The same key still closes with the session list focused, so what refused + // above was the guard and not a close that had stopped working. + after.focusContent() + closed, _ := after.dashboardKey(typed("x")) + done := closed.(Model) + if n := len(done.state.Sessions); n != 1 { + t.Errorf("sessions = %d after x with the session list focused, want 1", n) + } + // Naming the directory is the whole promise of keeping the worktree: a + // notice that only said "closed" would leave the work somewhere the user + // cannot find. Asserting the text, not just that there is some. + if !strings.Contains(done.notice, "/worktrees/sess") { + t.Errorf("notice = %q, want it to name where the worktree was kept", done.notice) + } +} + +// TestClosingTheLastSessionReleasesFocus keeps colContent meaning "a session is +// selected", which is what focusedSession's refusal depends on. +// +// focusContent refuses to focus an empty session list. Closing the last session +// reached that same state from the other side, and the next x then refused in +// silence — there was no session for the notice to be about. +func TestClosingTheLastSessionReleasesFocus(t *testing.T) { + t.Setenv("XDG_STATE_HOME", t.TempDir()) + m := modelWith(1) + m.focusContent() + if m.focus != colContent { + t.Fatal("could not focus the session list") + } + + m.closeSelectedFromDashboard() + + if m.focus != colProjects { + t.Errorf("focus = %v after closing the last session, want the projects column", m.focus) + } +} + +// TestConnectIgnoresProjectsColumn covers the other key that resolves through +// the cursor, and with it the notice surviving the route. +// +// dashboardKey takes a value receiver, so the refusal is written into a copy of +// the model that has to be the one returned. A far project exists here on +// purpose: without it the picker would refuse for its own reason and the test +// would pass while proving nothing. +func TestConnectIgnoresProjectsColumn(t *testing.T) { + m := modelWith(1) + far := m.state.AddProject(store.Project{Name: "other", Path: "/other"}) + m.state.AddSession(store.Session{ProjectID: far.ID, Name: "far", Title: "far"}) + m.rebuildRows() + + refused, _ := m.dashboardKey(typed("c")) + after := refused.(Model) + if after.picker != nil { + t.Error("c opened the connect picker from the projects list") + } + if after.notice == "" { + t.Error("the refused key said nothing about why") + } + + after.focusContent() + opened, _ := after.dashboardKey(typed("c")) + if opened.(Model).picker == nil { + t.Error("c did not open the picker with the session list focused") + } +} diff --git a/internal/ui/closing.go b/internal/ui/closing.go new file mode 100644 index 0000000..722f7c9 --- /dev/null +++ b/internal/ui/closing.go @@ -0,0 +1,47 @@ +package ui + +// Ending a session: stopping its agent, forgetting the record, and what is +// deliberately left on disk behind it. + +// closeSelectedFromDashboard stops a session's agent and forgets it. +// +// The worktree is left on disk on purpose. It may hold uncommitted work, and +// deleting a branch's only checkout to tidy a list is not a trade Deck +// gets to make silently. The notice says where it went. +func (m *Model) closeSelectedFromDashboard() { + p := m.currentProject() + selected := m.focusedSession() + if p == nil || selected == nil { + return + } + sess := *selected + if r, ok := m.runners[sess.ID]; ok { + r.Stop() + delete(m.runners, sess.ID) + } + m.releaseCoord(sess.ID) + // RemoveSession drops the links this session held, so the coordinator has + // to be told: releaseCoord frees claims and the inbox, but peers is set + // wholesale and outlives an agent exiting on purpose. + m.state.RemoveSession(sess.ID) + if err := m.state.Save(); err != nil { + m.fault = err + return + } + m.syncConnections() + m.rebuildRows() + left := m.state.SessionsFor(p.ID) + m.listIx = clamp(m.listIx, 0, max(len(left)-1, 0)) + // Closing the last one leaves the cursor in a column with nothing in it — + // the state focusContent refuses to create, reached from the other side. + // focusedSession's promise that a refusal says why rests on this: with no + // sessions there is nothing for it to name, so it would refuse in silence. + if len(left) == 0 { + m.focus = colProjects + } + if sess.Isolated { + m.notice = "closed " + sess.Name + " — worktree kept at " + sess.Dir + } else { + m.notice = "closed " + sess.Name + } +} diff --git a/internal/ui/keyroutes.go b/internal/ui/keyroutes.go index dd694cd..d32d852 100644 --- a/internal/ui/keyroutes.go +++ b/internal/ui/keyroutes.go @@ -31,7 +31,11 @@ func (m Model) dashboardKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { case key.Matches(msg, m.keys.Rename): return m.openEditProjectForm() case key.Matches(msg, m.keys.Connect): - return m.openConnectPicker(m.dashboardSession()) + // Resolved first, not inline: focusedSession writes its refusal notice + // into m, and Go does not order a method's receiver against a call in + // its own argument list. + sess := m.focusedSession() + return m.openConnectPicker(sess) case key.Matches(msg, m.keys.Theme): return m.openThemePicker() case key.Matches(msg, m.keys.Delete): diff --git a/internal/ui/sessions.go b/internal/ui/sessions.go index f613a41..bbf8df0 100644 --- a/internal/ui/sessions.go +++ b/internal/ui/sessions.go @@ -1,6 +1,7 @@ package ui -// Creating, selecting and closing sessions, and the worktrees behind them. +// Creating a session and the worktree behind it, and resolving which session +// the dashboard cursor is on. Ending one is closing.go. import ( "fmt" @@ -127,6 +128,9 @@ func (m Model) openFromDashboard() (tea.Model, tea.Cmd) { // session, while closing and connecting do nothing. That is the right split — // opening a session the cursor is near is helpful, closing or connecting one // the user cannot see is not — so do not fold them together. +// +// focusedSession is the third rule, and the one the keys that act on a single +// session go through. func (m Model) dashboardSession() *store.Session { p := m.currentProject() if p == nil { @@ -139,37 +143,25 @@ func (m Model) dashboardSession() *store.Session { return m.state.Session(sessions[m.listIx].ID) } -// closeSelectedFromDashboard stops a session's agent and forgets it. +// focusedSession is the session under the cursor, and only while the session +// list holds focus. // -// The worktree is left on disk on purpose. It may hold uncommitted work, and -// deleting a branch's only checkout to tidy a list is not a trade Deck -// gets to make silently. The notice says where it went. -func (m *Model) closeSelectedFromDashboard() { - p := m.currentProject() - selected := m.dashboardSession() - if p == nil || selected == nil { - return - } - sess := *selected - if r, ok := m.runners[sess.ID]; ok { - r.Stop() - delete(m.runners, sess.ID) - } - m.releaseCoord(sess.ID) - // RemoveSession drops the links this session held, so the coordinator has - // to be told: releaseCoord frees claims and the inbox, but peers is set - // wholesale and outlives an agent exiting on purpose. - m.state.RemoveSession(sess.ID) - if err := m.state.Save(); err != nil { - m.fault = err - return - } - m.syncConnections() - m.rebuildRows() - m.listIx = clamp(m.listIx, 0, max(len(m.state.SessionsFor(p.ID))-1, 0)) - if sess.Isolated { - m.notice = "closed " + sess.Name + " — worktree kept at " + sess.Dir - } else { - m.notice = "closed " + sess.Name +// dashboardSession answers whichever column has focus, so x and c acted on a +// session the keyboard was not driving — with the projects list focused, +// moveDashboard has reset listIx to 0, so it is whichever session is newest. +// The row is not invisible: cursorMarker keeps a dimmed cursor on the unfocused +// column on purpose. What is missing is the focus, and sectionLeft already +// scopes ←/→ by it for the reason it states — the focused column is drawn with +// an accent border, and a key that reaches across makes that border a lie. The +// footer says the same thing by omitting both keys there. Closing is where it +// costs most, because ←/→ is reversible and closing is not. +// +// The refusal says why. jumpToSession settled that a key which silently does +// nothing only invites a second press. +func (m *Model) focusedSession() *store.Session { + if m.focus != colContent { + m.notice = "no session selected — tab to the sessions list" + return nil } + return m.dashboardSession() }