fix(native): send a pull request's review comments to its own session

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Comments for the session written on a pull request's Files tab in Pulls go to a session in the checkout of that pull request's branch, where Send typed them into whichever session was open before Pulls, reviewing another branch. With no session there, Send says to open one and keeps the comments; Open in Session starts it. A line reference sent from that tab goes the same way.
 - On Windows with display scaling above 100%, a drag follows the pointer again: an item dragged in Settings › Footer or a selection dragged in the terminal landed away from the cursor, scaled as if the screen were at 100%.
 - Review comments sent to a session are kept until the session takes them. A session that refused the batch (one that had stopped, say) showed "Couldn't send the comments" while the comments were already gone; they now stay in the batch to send again. Likewise, submitting a pull request review in Pulls drops only the comments it carried, where a comment written while the review was on its way was lost with them.
 - On Windows, the window wears lich's icon and opens under lich's taskbar button, where it showed a generic icon and, launched from a lich pinned to the taskbar, opened as a separate button beside the pin.

native/cmd/lich-native/dock_comments.go

@@ -102,6 +102,21 @@ func (s *commentStore) edit(target string, id int, text string) {
 
 func (s *commentStore) clear(target string) { delete(s.byTarget, target) }
 
+// move hands from's batch to to, after what to already holds: a pull
+// request's comments, kept under its page while no checkout held its branch,
+// follow it into the one that now does.
+func (s *commentStore) move(from, to string) {
+	moved, ok := s.byTarget[from]
+	if !ok {
+		return
+	}
+	s.byTarget[to] = append(s.byTarget[to], moved...)
+	delete(s.byTarget, from)
+	if s.editing.target == from {
+		s.editing.target = to
+	}
+}
+
 func (s *commentStore) beginEdit(target string, id int) { s.editing = commentRef{target, id} }
 
 func (s *commentStore) endEdit() { s.editing = commentRef{} }

native/cmd/lich-native/dock_comments_test.go

@@ -131,3 +131,21 @@ func TestCommentEditFollowsTheCommentItWasOpenedOn(t *testing.T) {
 		t.Fatal("still editing after endEdit")
 	}
 }
+
+func TestCommentBatchMovesWholeAfterWhatItJoins(t *testing.T) {
+	var s commentStore
+	s.add("/wt", "a.go", "1", "already here")
+	s.add("https://pr/1", "b.go", "2", "written before the checkout")
+	s.beginEdit("https://pr/1", s.list("https://pr/1")[0].id)
+	s.move("https://pr/1", "/wt")
+	got := s.list("/wt")
+	if len(got) != 2 || got[0].txt != "already here" || got[1].txt != "written before the checkout" {
+		t.Fatalf("batch of /wt = %+v, want its own comment then the moved one", got)
+	}
+	if len(s.list("https://pr/1")) != 0 {
+		t.Fatalf("batch left behind = %+v, want none", s.list("https://pr/1"))
+	}
+	if c, ok := s.edited(); !ok || c.txt != "written before the checkout" {
+		t.Fatalf("edited = %+v, %v; want the moved comment still open", c, ok)
+	}
+}

native/cmd/lich-native/main.go

@@ -220,9 +220,9 @@ func run(w *app.Window, rt lichclient.Runtime) error {
 	u.palette = newCommandPalette(w.Invalidate)
 	u.worktree = newWorktreeDialog(client, &u.root, w.Invalidate)
 	u.pulls = newPullsScreen(newPullsStore(ctx, client, w.Invalidate), &u.root)
-	u.pulls.sessionAt = func(projectID, path string) bool {
+	u.pulls.sessionsIn = func(projectID, path string) []string {
 		ids, _, _ := u.model.sessionsAt(projectID, path)
-		return len(ids) > 0
+		return ids
 	}
 	u.model.prChanged = u.prChanged
 	u.model.themeSelection = u.appearance.applySelection
@@ -600,7 +600,7 @@ func (u *window) body(gtx C, v stageView) (int, sidebarAction, error) {
 						return D{Size: gtx.Constraints.Max}
 					}
 					if u.pulls.open && !v.home {
-						u.pulls.files = filesInput{sessionID: v.in.sessionID, inject: u.terms.injector(v.in.sessionID), submit: u.terms.submitter(v.in.sessionID), checkout: v.in.path, light: u.light, code: u.code}
+						u.pulls.files = filesInput{activeID: v.in.sessionID, injector: u.terms.injector, submitter: u.terms.submitter, light: u.light, code: u.code}
 						u.pulls.squareX = -1
 						if x, ok := u.topbar.islandSquare(gtx, topbarFrame{tabs: v.strip, active: v.stripActive, islands: u.pulls.spans, islandsSliding: u.pulls.slide.sliding()}); ok {
 							u.pulls.squareX = x

native/cmd/lich-native/pulls.go

@@ -3,6 +3,7 @@ package main
 import (
 	"image"
 	"image/color"
+	"slices"
 	"time"
 
 	"gioui.org/layout"
@@ -98,14 +99,29 @@ type pullsScreen struct {
 
 	view    pullView
 	confirm removeConfirm
-	// files is the window's active session and checkout, for the Files tab.
+	// files is what the Files tab draws with and sends through.
 	files filesInput
-	// sessionAt is whether a session of the project sits in a checkout.
-	sessionAt func(projectID, path string) bool
+	// sessionsIn is the project's sessions sitting in a checkout.
+	sessionsIn func(projectID, path string) []string
 }
 
-func (p *pullsScreen) hasSessionAt(path string) bool {
-	return p.sessionAt != nil && p.sessionAt(p.project.ID, path)
+// reviewSession is where the pull request's comments go: a session in the
+// checkout of its head branch, the window's active one when it sits there and
+// the first otherwise, "" when none does. target keys its comment batch: that checkout, shared with its
+// dock, or the PR's page while the branch has none.
+func (p *pullsScreen) reviewSession(d lichclient.PRDetail) (id, target string) {
+	wt, ok := p.checkedOut(d.HeadRefName)
+	if !ok {
+		return "", d.URL
+	}
+	ids := p.sessionsIn(p.project.ID, wt.Path)
+	switch {
+	case slices.Contains(ids, p.files.activeID):
+		return p.files.activeID, wt.Path
+	case len(ids) > 0:
+		return ids[0], wt.Path
+	}
+	return "", wt.Path
 }
 
 func newPullsScreen(store *pullsStore, root *ui.Root) *pullsScreen {

native/cmd/lich-native/pulls_files.go

@@ -64,14 +64,14 @@ type filesTree struct {
 }
 
 // filesInput is what the Files tab needs from the window this frame: the
-// active session, whose terminal takes injected references, and its
-// checkout, whose comment batch the tab shares with the dock.
+// active session, and how to inject into and submit to a session, bound to
+// the PR's own (reviewSession).
 type filesInput struct {
-	sessionID, checkout string
-	inject              func(string) bool
-	submit              func(string, func(error)) bool
-	light               bool
-	code                godemirror.Theme
+	activeID  string
+	injector  func(id string) func(string) bool
+	submitter func(id string) func(string, func(error)) bool
+	light     bool
+	code      godemirror.Theme
 }
 
 // viewedTick is the card's Viewed checkbox; ticking folds the file away.
@@ -132,8 +132,12 @@ func (v *pullView) filesTab(gtx C, th *ui.Theme, p *pullsScreen, d lichclient.PR
 		files.pr.head = d.Commits[len(d.Commits)-1].OID
 	}
 	in := p.files
-	files.th, files.light, files.code, files.target = th, in.light, in.code, in.checkout
-	files.inject, files.submit = in.inject, in.submit
+	session, target := p.reviewSession(d)
+	if target != d.URL {
+		files.comments.move(d.URL, target)
+	}
+	files.th, files.light, files.code, files.target = th, in.light, in.code, target
+	files.inject, files.submit = in.injector(session), in.submitter(session)
 	read := p.store.diff(p.project.Path, d.Number)
 	switch {
 	case !read.loaded:

native/cmd/lich-native/pulls_panels.go

@@ -68,7 +68,7 @@ func (v *pullView) actions(gtx C, th *ui.Theme, p *pullsScreen, d lichclient.PRD
 // sits in the branch's checkout.
 func (v *pullView) sessionButton(gtx C, th *ui.Theme, p *pullsScreen, d lichclient.PRDetail, busy, blocked string) D {
 	label := i18n.T("pulls.pulls.openInSession")
-	if wt, ok := p.checkedOut(d.HeadRefName); ok && p.hasSessionAt(wt.Path) {
+	if id, _ := p.reviewSession(d); id != "" {
 		label = i18n.T("pulls.pulls.goToSession")
 	}
 	if busy == "session" {

native/cmd/lich-native/pulls_store_test.go

@@ -109,3 +109,36 @@ func TestPullsScreenRereadsWhatItShowsEveryRecheck(t *testing.T) {
 	p.store.list("/repo", pullsOpen)
 	waitFor(t, "the recheck", func() bool { return stub.calls("project.ListPullRequests") == 2 })
 }
+
+func TestPullRequestCommentsGoOnlyToASessionInItsBranchCheckout(t *testing.T) {
+	stub := &rpcStub{answer: func(method string, _ []json.RawMessage) (any, int) {
+		if method == "project.ListCheckouts" {
+			return []lichclient.Worktree{{Name: "main", Path: "/repo"}, {Name: "feat/x", Path: "/wt/feat-x"}}, http.StatusOK
+		}
+		return []lichclient.PRSummary{}, http.StatusOK
+	}}
+	p := newPullsScreen(newPullsStore(context.Background(), stub.client(t), func() {}), new(ui.Root))
+	p.show(lichclient.Project{ID: "p", Path: "/repo"}, "main", 0)
+	p.sessionsIn = func(projectID, path string) []string {
+		if projectID == "p" && path == "/wt/feat-x" {
+			return []string{"in-branch", "second"}
+		}
+		return []string{"elsewhere"}
+	}
+	waitFor(t, "the checkouts", func() bool { return len(p.store.checkoutsOf("/repo")) == 2 })
+
+	if id, target := p.reviewSession(lichclient.PRDetail{HeadRefName: "feat/x", URL: "https://pr/1"}); id != "in-branch" || target != "/wt/feat-x" {
+		t.Errorf("checked-out branch: session %q, target %q; want in-branch, /wt/feat-x", id, target)
+	}
+	p.files.activeID = "second"
+	if id, _ := p.reviewSession(lichclient.PRDetail{HeadRefName: "feat/x"}); id != "second" {
+		t.Errorf("active session in the checkout: session %q, want second", id)
+	}
+	p.files.activeID = "elsewhere"
+	if id, _ := p.reviewSession(lichclient.PRDetail{HeadRefName: "feat/x"}); id != "in-branch" {
+		t.Errorf("active session in another checkout: session %q, want in-branch", id)
+	}
+	if id, target := p.reviewSession(lichclient.PRDetail{HeadRefName: "feat/y", URL: "https://pr/2"}); id != "" || target != "https://pr/2" {
+		t.Errorf("branch with no checkout: session %q, target %q; want none, the PR's page", id, target)
+	}
+}

native/cmd/lich-native/stale_pulls_test.go

@@ -38,6 +38,11 @@ func pullsFrames(t *testing.T) (*window, *frameRig) {
 	client := stub.client(t)
 	u := &window{ctx: t.Context(), client: client, model: newModel(client, r.invalidate)}
 	u.pulls = newPullsScreen(newPullsStore(t.Context(), client, r.invalidate), &u.root)
+	u.pulls.sessionsIn = func(string, string) []string { return nil }
+	u.pulls.files = filesInput{
+		injector:  func(string) func(string) bool { return func(string) bool { return false } },
+		submitter: func(string) func(string, func(error)) bool { return func(string, func(error)) bool { return false } },
+	}
 	files, err := newReviewPanel(client, &u.root, new(commentStore), "monospace", 12, r.invalidate, func(ui.Toast) {})
 	if err != nil {
 		t.Fatal(err)