From 8c5a156b6482c17fc69007f0b6712ca55e927445 Mon Sep 17 00:00:00 2001 From: tdwd <111124579+tdwd@users.noreply.github.com> Date: Mon, 7 Sep 2026 18:16:39 +0200 Subject: [PATCH 1/3] Move session closing into its own file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sessions.go held creating, selecting and closing. The next commit adds a resolver beside the other two and would take the file past the 200-line rule, so the split comes first and on its own — a refactor is unreviewable once a behaviour change is hiding in it. closing.go is named for the category rather than its occupant. Deleting a worktree, when it exists, belongs here too. The doc comment travels with the function and no copy stays behind; sessions.go's header now says where closing went rather than continuing to claim it. --- internal/ui/closing.go | 39 +++++++++++++++++++++++++++++++++++++++ internal/ui/sessions.go | 38 ++------------------------------------ 2 files changed, 41 insertions(+), 36 deletions(-) create mode 100644 internal/ui/closing.go diff --git a/internal/ui/closing.go b/internal/ui/closing.go new file mode 100644 index 0000000..d6fde9c --- /dev/null +++ b/internal/ui/closing.go @@ -0,0 +1,39 @@ +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.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 + } +} diff --git a/internal/ui/sessions.go b/internal/ui/sessions.go index f613a41..c86d806 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" @@ -138,38 +139,3 @@ func (m Model) dashboardSession() *store.Session { } return m.state.Session(sessions[m.listIx].ID) } - -// 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.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 - } -} From d7002f5541e8c422893b028ab2e4e042e399e012 Mon Sep 17 00:00:00 2001 From: tdwd <111124579+tdwd@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:59:24 +0200 Subject: [PATCH 2/3] Refuse x and c unless the session list has focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dashboardKey routed both keys whatever column had focus, while dashboardSession resolves through listIx. Pressing x on the projects list therefore 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, and there is a test named for that. What was missing was the focus. sectionLeft already scopes the arrow keys by it and states the reason — the focused column is drawn with an accent border, and a key that reaches across makes that border a lie. The footer said the same by omitting both keys there, so x was destructive and undocumented in the same place. Closing is where it costs most, because the arrows are reversible and closing is not. focusedSession is where the rule lives, and the refusal says why: jumpToSession settled that a key which silently does nothing only invites a second press. Closing the last session is the same invariant from the other side, and it goes here rather than in its own commit because the refusal above depends on it. focusContent refuses to focus an empty list, but closing reached that state anyway — the cursor sat in a column with nothing in it, and the next x refused in silence because there was no session for the notice to name. Focus now returns to the projects column when a project runs out of sessions. The connect key resolves into a variable rather than inline, because focusedSession writes its notice into m and Go does not order a method's receiver against a call in its own argument list. The close test now asserts the notice names the worktree directory rather than merely being non-empty. Keeping the worktree is only useful if you are told where it went, and that promise had no test at all. --- README.md | 4 ++ internal/ui/app_test.go | 92 ++++++++++++++++++++++++++++++++++++++++ internal/ui/closing.go | 12 +++++- internal/ui/keyroutes.go | 6 ++- internal/ui/sessions.go | 26 ++++++++++++ 5 files changed, 137 insertions(+), 3 deletions(-) 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/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 index d6fde9c..722f7c9 100644 --- a/internal/ui/closing.go +++ b/internal/ui/closing.go @@ -10,7 +10,7 @@ package ui // gets to make silently. The notice says where it went. func (m *Model) closeSelectedFromDashboard() { p := m.currentProject() - selected := m.dashboardSession() + selected := m.focusedSession() if p == nil || selected == nil { return } @@ -30,7 +30,15 @@ func (m *Model) closeSelectedFromDashboard() { } m.syncConnections() m.rebuildRows() - m.listIx = clamp(m.listIx, 0, max(len(m.state.SessionsFor(p.ID))-1, 0)) + 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 { 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 c86d806..bbf8df0 100644 --- a/internal/ui/sessions.go +++ b/internal/ui/sessions.go @@ -128,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,3 +142,26 @@ func (m Model) dashboardSession() *store.Session { } return m.state.Session(sessions[m.listIx].ID) } + +// focusedSession is the session under the cursor, and only while the session +// list holds focus. +// +// 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() +} From 436beceb46b6af6946549d18be7810a27e01156d Mon Sep 17 00:00:00 2001 From: tdwd <111124579+tdwd@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:59:24 +0200 Subject: [PATCH 3/3] Record deleting a session in the backlog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing keeps the worktree, which is the right default and currently the only one. Nothing in Deck removes what it keeps, so a machine accumulates a tree and a session/* branch per closed isolated session, and git by hand is the only way out. The entry settles the shape: x opens a modal with both outcomes rather than growing a second key, Close stays the default, and the Delete row carries the gitx.Diff summary so the confirmation states a fact rather than a warning. Four decisions go with it, and the one most likely to be softened later is that nothing is forced — git worktree remove refuses a dirty tree and git branch -d refuses unmerged commits, and both refusals are the answer. Verified rather than assumed: in a throwaway repository, removing a dirty worktree gives "contains modified or untracked files, use --force", while a clean worktree whose branch holds unmerged commits removes fine and then branch -d refuses. That is exactly the gate-and-best-effort split the entry describes. No code changes. --- docs/backlog.md | 61 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) 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