Delete a session, and the worktree it leaves

README.md

@@ -114,7 +114,7 @@ the arrows keep working inside `claude`.
 | `n` | new session |
 | `a` | add project |
 | `e` | rename project |
-| `x` | close session |
+| `x` | end session — close and keep the worktree, or delete both |
 | `c` | connect this session to one on another project |
 | `t` | theme picker |
 | `?` | help |

docs/architecture.md

@@ -56,7 +56,7 @@ see its siblings.
 ```
 main.go            flags, state load, cwd registration, teardown after Run
 internal/naming    scheming-hawk-jhgk names, branch names, slugs
-internal/gitx      repo root, branch, worktree add
+internal/gitx      repo root, branch, worktree add and remove
 internal/termquery answers terminal queries for harnesses with no terminal
 internal/store     projects + sessions, atomic JSON persistence:
                      store.go      the document, load and save
@@ -90,14 +90,16 @@ internal/ui        the Bubble Tea program, split by job:
                      keys.go       the binding table; ptykeys.go encodes to bytes
                      selection.go  cursor and list navigation
                      actions.go    forms; sessions.go and projects.go do the work
+                     closing.go    ending a session: close keeps the worktree,
+                                   delete removes it and the branch
                      runner.go     agent lifecycle; agentargs.go builds its argv
                      dashboard.go  + projectlist.go / projectdetail.go / help.go
                      session.go    + chrome.go / sidebar.go / status.go / pane.go
                      form.go       the modal widget; formsession.go and
                                    formproject.go build the forms,
                                    formfields.go the pieces, forminput.go keys
                      browser.go    the directory explorer; dirlist.go lists it
-                     picker.go     the list modal; picker_open.go opens it
+                     picker.go     the list modal; pickerctl.go opens and drives it
                      theme.go      + palette.go / styles.go
 internal/gittest   throwaway git repositories for tests, in one place because
                    four packages had grown their own and they had drifted
@@ -106,8 +108,12 @@ probe/             debug harness: renders frames without a human present
 
 `gitx` holds only what something calls. It briefly carried `IsDirty`,
 `AheadBehind`, `HeadSubject`, `RemoveWorktree` and `DeleteBranch` because a git
-wrapper "should" have them; all five had tests and no caller, which is how
-dead code passes review. If a feature needs one, add it back with the feature.
+wrapper "should" have them; all five had tests and no caller, which is how dead
+code passes review. If a feature needs one, add it back with the feature.
+
+`RemoveWorktree` and `DeleteBranch` came back that way, with the `x` modal that
+calls them. The other three are still absent, and having a test is still not
+having a caller.
 
 ### Relationship to cathode
 
@@ -541,13 +547,21 @@ screen and one keystroke from the right directory, so an error belongs there.
 A pick that survives opens the project form with the path prefilled and focus
 on the description.
 
-### One list widget, two jobs (`ui/picker.go`)
+### One list widget, four jobs (`ui/picker.go`)
+
+`picker` is a modal list with a `pickerKind`. It serves the theme list, any form
+field marked `pickable` — currently the project field — the connect list, and
+the question `x` asks about a worktree. The kinds behave differently on purpose.
+The theme picker previews by applying and persists on commit. A field picker
+floats over the open form, previews nothing, and only writes the chosen value
+back into the field. Connecting and ending both act on commit alone, because a
+link is a change to the store and ending a session is not reversible, so
+previewing either as the cursor moves would do the thing being asked about.
 
-`picker` is a modal list with a `pickerKind`. It serves the theme list and any
-form field marked `pickable` — currently the project field. The kinds behave
-differently on purpose: the theme picker previews by applying and persists on
-commit, while a field picker floats over the open form, previews nothing, and
-only writes the chosen value back into the field.
+`pickerSubject` holds the session an open picker is about, captured when it
+opens rather than read back on the commit key. The modal outlives the keystroke
+that opened it, and the dashboard and the session view resolve "the selected
+session" by different rules.
 
 The form's own choice field renders options as a row of chips and falls back to
 a one-at-a-time cycler when they overflow. That is right for two working-copy
@@ -674,6 +688,44 @@ switching to the project directory is one `tab` away. Check `gitx.HasCommits`
 before anything else that assumes a resolvable HEAD — git's own "fatal: invalid
 reference: HEAD" is accurate and useless.
 
+### Ending a session is two outcomes (`ui/closing.go`)
+
+`x` opens a modal rather than acting. **Close** forgets the record and leaves
+the worktree on disk. **Delete** removes the worktree and the branch. Close is
+the row the cursor starts on, because it is the reversible one and a modal that
+opens on the destructive row turns a confirmation into a trap.
+
+The Delete row carries what the session changed — "3 files changed, 41
+insertions(+)" — measured by `gitx.Diff` against `Session.BaseRef`. A
+confirmation that states a fact can be answered. One that states a warning can
+only be believed or dismissed. A failed measurement says so in place of the
+figure, because a row reading "no files changed" because git errored would state
+a fact it does not have.
+
+**Nothing is forced.** `git worktree remove` refuses a tree holding modified or
+untracked files and `git branch -d` refuses unmerged commits, and both refusals
+are the answer rather than an obstacle. Untracked counts, which is the case that
+surprises: an agent that wrote a file and never added it leaves the tree dirty
+with nothing in `git diff` to show for it. The session stays, so the way out is
+to open it again, commit the work, and delete it after.
+
+**The worktree is the gate, the branch is best effort.** A clean tree whose
+branch holds unmerged commits removes fine, and `branch -d` then refuses. The
+session goes and the work stays, which is the right outcome, so the notice names
+the branch that was kept.
+
+**A session that ran in the project directory has nothing to delete.** Its `Dir`
+is the project's own checkout, so it skips the modal and closes outright.
+`deleteSession` guards that itself rather than trusting its caller, because the
+cost of reaching it with one is git aimed at the user's own tree.
+
+The disk work comes first and `forget` comes last, so a worktree that refused to
+go still has a row naming it. `forget` is not itself atomic: `RemoveSession`
+runs before `Save`, so a failed save leaves the session gone from memory and
+still in `state.json`, where it returns at the next launch. Putting it back is
+not possible from there, because `RemoveSession` also drops the links the
+session held. Making the pair atomic belongs in `store`.
+
 ### An exited agent is a restart, not a dead end
 
 `landOn` drops the attachment when the cursor moves to a session with no

docs/backlog.md

@@ -236,64 +236,25 @@ 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.
+## ~~14. Deleting a session, and the worktree it leaves~~ — done 2026-09-18
+
+Shipped as **Ending a session is two outcomes** in `docs/architecture.md`, with
+`gitx.RemoveWorktree` and `gitx.DeleteBranch` underneath it.
+
+`x` opens a modal holding both outcomes rather than growing a second key. Close
+is the default because it is the reversible one. The Delete row carries
+`gitx.Diff` against `BaseRef`, so the confirmation states a fact instead of a
+warning.
+
+All four decisions this entry was waiting on held once they met real git, and
+one of them turned out wider than written. `git worktree remove` refuses a tree
+holding **untracked** files, not only modified ones, so an agent that wrote a
+file and never added it is enough to stop a delete. That is the right refusal,
+and it is a second way to reach it that this entry did not name.
+
+Deliberately still 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
 

internal/gitx/gitx.go

@@ -4,6 +4,11 @@
 // guaranteed to agree with the user's own `git worktree list`.
 package gitx
 
+// Running git at all, and the questions asked of a repository before Deck
+// commits to anything: where its root is, what it has checked out, whether it
+// has a commit yet, and whether a directory is a project or a shelf of them.
+// The worktree a session gets is worktree.go.
+
 import (
 	"bytes"
 	"errors"
@@ -17,11 +22,6 @@ import (
 // ErrNotARepo is returned when a path is not inside a git working tree.
 var ErrNotARepo = errors.New("not a git repository")
 
-// ErrNoCommits is returned when a repository has an unborn HEAD — freshly
-// initialised, nothing committed. There is no commit for a worktree to check
-// out, so an isolated session is impossible until the first commit exists.
-var ErrNoCommits = errors.New("repository has no commits yet")
-
 // run executes git in dir and returns trimmed stdout. git writes its
 // diagnostics to stderr, so the error carries them verbatim — a wrapped
 // "exit status 128" on its own tells the user nothing.
@@ -122,34 +122,3 @@ func HoldsRepos(dir string) bool {
 func HeadCommit(dir string) (string, error) {
 	return run(dir, "rev-parse", "HEAD")
 }
-
-// AddWorktree creates branch at the current HEAD of repo and checks it out
-// into dest. dest must not exist; git refuses to reuse a populated directory
-// and we do not try to talk it round.
-func AddWorktree(repo, dest, branch string) error {
-	// A project need not be a repository — it may be a directory that only
-	// collects them. Say that plainly rather than letting the unborn-HEAD
-	// check below report "no commits yet", which is true of a non-repository
-	// and tells the user nothing about what is actually wrong.
-	if _, err := RepoRoot(repo); err != nil {
-		// Wrapping prepends the sentinel's own text, so this half must not
-		// repeat it or the message reads "not a git repository: X is not a
-		// git repository".
-		return fmt.Errorf("%w: %s has no branch to work from — open the session in the project directory instead",
-			ErrNotARepo, filepath.Base(repo))
-	}
-	// Check for an unborn HEAD next. Without this the caller gets git's own
-	// "fatal: invalid reference: HEAD", which is accurate and tells a user
-	// nothing about what to do next.
-	if !HasCommits(repo) {
-		return fmt.Errorf("%w: commit something first, or open the session in the project directory", ErrNoCommits)
-	}
-	if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
-		return fmt.Errorf("create worktree parent: %w", err)
-	}
-	if _, err := os.Stat(dest); err == nil {
-		return fmt.Errorf("worktree path already exists: %s", dest)
-	}
-	_, err := run(repo, "worktree", "add", "-b", branch, dest, "HEAD")
-	return err
-}

internal/gitx/gitx_test.go

@@ -50,77 +50,25 @@ func TestHeadBranch(t *testing.T) {
 	}
 }
 
-// TestAddWorktree covers what a session needs: a checkout of its own, on its
-// own branch.
-//
-// There is no removal half. Deck deliberately leaves a worktree on disk
-// when a session closes, because it may hold uncommitted work — so there is no
-// remove function to test, and adding one for the test's sake would be code
-// with no caller.
-func TestAddWorktree(t *testing.T) {
-	repo := testRepo(t)
-	dest := filepath.Join(t.TempDir(), "worktrees", "scheming-hawk-jhgk")
-	const branch = "session/scheming-hawk-jhgk"
-
-	if err := AddWorktree(repo, dest, branch); err != nil {
-		t.Fatalf("AddWorktree: %v", err)
-	}
-	if _, err := os.Stat(filepath.Join(dest, ".git")); err != nil {
-		t.Fatalf("worktree has no .git: %v", err)
-	}
-	got, err := HeadBranch(dest)
-	if err != nil {
-		t.Fatalf("HeadBranch in worktree: %v", err)
-	}
-	if got != branch {
-		t.Errorf("worktree branch = %q, want %q", got, branch)
-	}
-}
-
-// TestAddWorktreeRefusesExistingPath matters because silently reusing a
-// populated directory would put an agent to work in someone else's tree.
-func TestAddWorktreeRefusesExistingPath(t *testing.T) {
-	repo := testRepo(t)
-	dest := filepath.Join(t.TempDir(), "taken")
-	if err := os.MkdirAll(dest, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if err := AddWorktree(repo, dest, "session/x"); err == nil {
-		t.Fatal("AddWorktree overwrote an existing path")
-	}
-}
-
 // TestRunErrorCarriesGitStderr keeps the diagnostics: a bare "exit status 128"
 // tells the user nothing about what git objected to.
+//
+// It asserts on git's own words rather than on the branch name, because run
+// formats the message as "git <args>: <stderr>" and the args already contain
+// the branch. The earlier assertion was that the message mentioned "main",
+// which stayed green with the stderr read replaced by err.Error() — measured,
+// not reasoned about. The negative assertion is the discriminating one.
 func TestRunErrorCarriesGitStderr(t *testing.T) {
 	repo := testRepo(t)
 	err := AddWorktree(repo, filepath.Join(t.TempDir(), "wt"), "main")
 	if err == nil {
 		t.Fatal("creating a branch that already exists succeeded")
 	}
-	if !strings.Contains(err.Error(), "main") {
-		t.Errorf("error does not mention the branch: %v", err)
+	if strings.Contains(err.Error(), "exit status") {
+		t.Errorf("error fell back to the exit code: %v", err)
 	}
-}
-
-// TestAddWorktreeOnUnbornHead covers a freshly `git init`-ed repository. git's
-// own message is "fatal: invalid reference: HEAD", which is accurate and tells
-// a user nothing about what to do next.
-func TestAddWorktreeOnUnbornHead(t *testing.T) {
-	// gittest.Repo initialises without committing, which is the unborn HEAD
-	// this test is about.
-	dir := gittest.Repo(t)
-
-	if HasCommits(dir) {
-		t.Fatal("a repository with no commits reported HasCommits")
-	}
-
-	err := AddWorktree(dir, filepath.Join(t.TempDir(), "wt"), "session/x")
-	if !errors.Is(err, ErrNoCommits) {
-		t.Fatalf("error = %v, want ErrNoCommits", err)
-	}
-	if !strings.Contains(err.Error(), "project directory") {
-		t.Errorf("error does not suggest the way out: %v", err)
+	if !strings.Contains(err.Error(), "already exists") {
+		t.Errorf("error does not carry git's complaint: %v", err)
 	}
 }
 
@@ -130,25 +78,6 @@ func TestHasCommits(t *testing.T) {
 	}
 }
 
-// TestAddWorktreeOnNonRepo covers a project that is a collector of
-// repositories rather than one itself. Without the explicit check the
-// unborn-HEAD branch reports "no commits yet", which is true of a
-// non-repository and explains nothing.
-func TestAddWorktreeOnNonRepo(t *testing.T) {
-	plain := t.TempDir()
-
-	err := AddWorktree(plain, filepath.Join(t.TempDir(), "wt"), "session/x")
-	if !errors.Is(err, ErrNotARepo) {
-		t.Fatalf("error = %v, want ErrNotARepo", err)
-	}
-	if strings.Contains(err.Error(), "no commits") {
-		t.Errorf("a non-repository was reported as having no commits: %v", err)
-	}
-	if !strings.Contains(err.Error(), "project directory") {
-		t.Errorf("error does not suggest the way out: %v", err)
-	}
-}
-
 // TestHoldsRepos covers the collector check that lets `deck` seed a directory
 // which is not itself a repository but coordinates several that are.
 func TestHoldsRepos(t *testing.T) {
@@ -165,9 +94,15 @@ func TestHoldsRepos(t *testing.T) {
 		t.Error("a directory holding a repository was not recognised")
 	}
 
-	// One level only: a grandparent is not a collector, or $HOME would be.
-	if HoldsRepos(filepath.Dir(collector)) == HoldsRepos(collector) {
-		return // sibling temp dirs may coincidentally contain repos; not asserting
+	// One level only: a grandparent is not a collector, or $HOME would be. The
+	// tree is built here rather than read from filepath.Dir(collector), whose
+	// contents the test does not control.
+	deep := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(deep, "mid", "repo", ".git"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if HoldsRepos(deep) {
+		t.Error("a repository two levels down made its grandparent a collector")
 	}
 }