fix(browse): refresh correct column after async document copy (#54)

The copiedMsg handler re-derived its refresh target from the live
m.activeColumn. Navigation keys are not gated on m.loading, so moving
columns during the async copy round-trip left activeColumn pointing at
an unrelated column by the time copiedMsg arrived — refreshing the wrong
Miller column (or none).

Capture the originating collection path in copiedMsg at copy time and
match it to a column in the handler via a new copyRefreshIndex helper,
instead of reading live state. If that column was navigated away, refresh
nothing. Adds a unit test exercising the race (active column moved /
origin column removed) and asserts collectionPath in the integration
tests.

Also drops the unused fromDocView parameter from executeCopy and the
now-dead m.copyFromDocView field that only fed it.

Co-authored-by: Gustavo Hoirisch <355877+gugahoi@users.noreply.github.com>

Author: gugahoi

pkg/cmd/browse/command.go

@@ -473,18 +473,15 @@ func cmdCopy(m *Model, args []string) (string, error) {
 
 	col := m.columns[m.activeColumn]
 	var src string
-	var fromDocView bool
 
 	if col.isDoc {
 		src = col.path
-		fromDocView = true
 	} else {
 		item := m.getSelectedItem()
 		if item == nil || !item.isDoc {
 			return "", fmt.Errorf("select a document to copy")
 		}
 		src = item.path
-		fromDocView = false
 	}
 
 	dst := ""
@@ -497,9 +494,8 @@ func cmdCopy(m *Model, args []string) (string, error) {
 	}
 
 	m.copySrc = src
-	m.copyFromDocView = fromDocView
 	m.loading = true
-	m.pendingCmd = executeCopy(m.client, src, dst, fromDocView)
+	m.pendingCmd = executeCopy(m.client, src, dst)
 	return "", nil
 }
 

pkg/cmd/browse/copy.go

@@ -13,11 +13,15 @@ import (
 )
 
 // copiedMsg is emitted once the source document's field data has been copied
-// into a new document. newPath is the path of the freshly created document and
-// hadSubcollections reports whether the source had subcollections (which are
-// not copied) so the caller can surface a note.
+// into a new document. newPath is the path of the freshly created document,
+// collectionPath is the collection the new sibling now lives in (captured at
+// copy time so the handler can refresh the correct column even if the user
+// navigated during the async copy), and hadSubcollections reports whether the
+// source had subcollections (which are not copied) so the caller can surface a
+// note.
 type copiedMsg struct {
 	newPath           string
+	collectionPath    string
 	hadSubcollections bool
 }
 
@@ -33,7 +37,7 @@ type copyRefusedMsg struct {
 // otherwise dst is used and the copy is refused if a document already exists
 // there. Only top-level field data is copied — subcollections are left behind.
 // The source is never modified.
-func executeCopy(client *firestore.Client, src, dst string, fromDocView bool) tea.Cmd {
+func executeCopy(client *firestore.Client, src, dst string) tea.Cmd {
 	return func() tea.Msg {
 		ctx := context.Background()
 		srcRef := client.Doc(strings.TrimPrefix(src, "/"))
@@ -64,7 +68,8 @@ func executeCopy(client *firestore.Client, src, dst string, fromDocView bool) te
 			if _, err := dstRef.Set(ctx, snap.Data()); err != nil {
 				return errorMsg{err: fmt.Errorf("failed to write copy: %w", err)}
 			}
-			return copiedMsg{newPath: parent + "/" + dstRef.ID, hadSubcollections: hadSub}
+			newPath := parent + "/" + dstRef.ID
+			return copiedMsg{newPath: newPath, collectionPath: parentCollectionPath(newPath), hadSubcollections: hadSub}
 		}
 
 		// Explicit destination — refuse if something is already there.
@@ -75,7 +80,8 @@ func executeCopy(client *firestore.Client, src, dst string, fromDocView bool) te
 			}
 			return errorMsg{err: fmt.Errorf("failed to write copy: %w", err)}
 		}
-		return copiedMsg{newPath: strings.Trim(dst, "/"), hadSubcollections: hadSub}
+		newPath := strings.Trim(dst, "/")
+		return copiedMsg{newPath: newPath, collectionPath: parentCollectionPath(newPath), hadSubcollections: hadSub}
 	}
 }
 
@@ -90,3 +96,25 @@ func parentCollectionPath(docPath string) string {
 	}
 	return p[:idx]
 }
+
+// copyRefreshIndex returns the index of the collection column that should be
+// refreshed after a copy completes, identified by the collection path captured
+// in copiedMsg at copy-initiation time. It deliberately does NOT consult
+// m.activeColumn: the user may have navigated during the async copy, so the
+// live active column can no longer be trusted to point at the originating
+// collection. Returns -1 when no current column matches (e.g. the column was
+// removed via navigation), in which case nothing should be refreshed.
+func (m Model) copyRefreshIndex(collectionPath string) int {
+	if collectionPath == "" {
+		return -1
+	}
+	for idx, col := range m.columns {
+		if col.isDoc {
+			continue
+		}
+		if strings.TrimPrefix(col.path, "/") == collectionPath {
+			return idx
+		}
+	}
+	return -1
+}

pkg/cmd/browse/copy_integration_test.go

@@ -19,14 +19,17 @@ func TestExecuteCopy_AutoID(t *testing.T) {
 		t.Fatalf("seed: %v", err)
 	}
 
-	msg := executeCopy(client, src, "", false)()
+	msg := executeCopy(client, src, "")()
 	copied, ok := msg.(copiedMsg)
 	if !ok {
 		t.Fatalf("expected copiedMsg, got %#v", msg)
 	}
 	if copied.newPath == src {
 		t.Fatalf("auto copy reused source path %s", src)
 	}
+	if copied.collectionPath != "copy_users" {
+		t.Errorf("collectionPath = %q, want %q", copied.collectionPath, "copy_users")
+	}
 	if copied.hadSubcollections {
 		t.Error("hadSubcollections = true, want false")
 	}
@@ -55,14 +58,17 @@ func TestExecuteCopy_ExplicitID(t *testing.T) {
 		t.Fatalf("seed: %v", err)
 	}
 
-	msg := executeCopy(client, src, dst, false)()
+	msg := executeCopy(client, src, dst)()
 	copied, ok := msg.(copiedMsg)
 	if !ok {
 		t.Fatalf("expected copiedMsg, got %#v", msg)
 	}
 	if copied.newPath != dst {
 		t.Errorf("newPath = %q, want %q", copied.newPath, dst)
 	}
+	if copied.collectionPath != "copy_users" {
+		t.Errorf("collectionPath = %q, want %q", copied.collectionPath, "copy_users")
+	}
 
 	// Both source and copy exist with the same data.
 	if _, err := client.Doc(src).Get(ctx); err != nil {
@@ -90,7 +96,7 @@ func TestExecuteCopy_RefusesExistingDestination(t *testing.T) {
 		t.Fatalf("seed dst: %v", err)
 	}
 
-	msg := executeCopy(client, src, dst, false)()
+	msg := executeCopy(client, src, dst)()
 	if _, ok := msg.(copyRefusedMsg); !ok {
 		t.Fatalf("expected copyRefusedMsg for existing destination, got %#v", msg)
 	}
@@ -119,7 +125,7 @@ func TestExecuteCopy_PhantomDocument(t *testing.T) {
 
 	// A phantom source has no field data, so copy is refused with a transient
 	// message rather than a sticky error.
-	msg := executeCopy(client, src, "", false)()
+	msg := executeCopy(client, src, "")()
 	if _, ok := msg.(copyRefusedMsg); !ok {
 		t.Fatalf("expected copyRefusedMsg for phantom document, got %#v", msg)
 	}
@@ -138,7 +144,7 @@ func TestExecuteCopy_IgnoresSubcollections(t *testing.T) {
 		t.Fatalf("seed subcollection: %v", err)
 	}
 
-	msg := executeCopy(client, src, dst, false)()
+	msg := executeCopy(client, src, dst)()
 	copied, ok := msg.(copiedMsg)
 	if !ok {
 		t.Fatalf("expected copiedMsg, got %#v", msg)

pkg/cmd/browse/copy_test.go

@@ -0,0 +1,94 @@
+package browse
+
+import "testing"
+
+// TestCopyRefreshIndex verifies that the post-copy refresh target is resolved
+// from the collection path captured at copy time, NOT from the live
+// activeColumn. This is the regression guard for the race where the user
+// navigates during an in-flight async copy: by the time copiedMsg arrives,
+// activeColumn no longer points at the originating collection.
+func TestCopyRefreshIndex(t *testing.T) {
+	tests := []struct {
+		name           string
+		columns        []Column
+		activeColumn   int
+		collectionPath string
+		want           int
+	}{
+		{
+			name: "origin collection found regardless of active column",
+			columns: []Column{
+				{path: "users", isDoc: false},
+				{path: "users/alice", isDoc: true},
+				{path: "users/alice/orders", isDoc: false},
+			},
+			// User navigated two columns to the right during the copy.
+			activeColumn:   2,
+			collectionPath: "users",
+			want:           0,
+		},
+		{
+			name: "origin column removed by navigation -> no refresh",
+			columns: []Column{
+				{path: "products", isDoc: false},
+			},
+			// The originating "users" collection is no longer on screen.
+			activeColumn:   0,
+			collectionPath: "users",
+			want:           -1,
+		},
+		{
+			name: "matches collection column, not same-path document column",
+			columns: []Column{
+				{path: "users", isDoc: false},
+				{path: "users/alice", isDoc: true},
+			},
+			activeColumn:   1,
+			collectionPath: "users",
+			want:           0,
+		},
+		{
+			name: "tolerates leading slash on column path",
+			columns: []Column{
+				{path: "/users", isDoc: false},
+			},
+			activeColumn:   0,
+			collectionPath: "users",
+			want:           0,
+		},
+		{
+			name: "empty collection path -> no refresh",
+			columns: []Column{
+				{path: "users", isDoc: false},
+			},
+			activeColumn:   0,
+			collectionPath: "",
+			want:           -1,
+		},
+		{
+			name: "nested subcollection origin",
+			columns: []Column{
+				{path: "users", isDoc: false},
+				{path: "users/alice", isDoc: true},
+				{path: "users/alice/orders", isDoc: false},
+			},
+			// Copy of a doc inside the orders subcollection; active column has
+			// since moved back to the root.
+			activeColumn:   0,
+			collectionPath: "users/alice/orders",
+			want:           2,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			m := Model{
+				columns:      tt.columns,
+				activeColumn: tt.activeColumn,
+			}
+			if got := m.copyRefreshIndex(tt.collectionPath); got != tt.want {
+				t.Errorf("copyRefreshIndex(%q) = %d, want %d", tt.collectionPath, got, tt.want)
+			}
+		})
+	}
+}

pkg/cmd/browse/model.go

@@ -14,7 +14,7 @@ import (
 type Mode int
 
 const (
-	ModeNormal  Mode = iota
+	ModeNormal Mode = iota
 	ModeVisual
 	ModeCommand
 )
@@ -60,8 +60,8 @@ type Model struct {
 	err          error
 
 	// Mode system
-	mode    Mode
-	overlay Overlay
+	mode      Mode
+	overlay   Overlay
 	textInput textinput.Model
 
 	// Command mode
@@ -107,8 +107,8 @@ type Model struct {
 	previewPending string // path of pending debounced fetch
 
 	// Delete confirmation
-	deletePath        string // Path of document pending deletion
-	deleteFromDocView bool   // Whether delete was initiated from a document column
+	deletePath        string   // Path of document pending deletion
+	deleteFromDocView bool     // Whether delete was initiated from a document column
 	bulkDeletePaths   []string // Paths for bulk delete in visual mode
 
 	// Rename / move
@@ -117,8 +117,7 @@ type Model struct {
 	renameFromDocView bool   // Whether rename was initiated from a document column
 
 	// Copy / duplicate
-	copySrc         string // Source path of document being copied
-	copyFromDocView bool   // Whether copy was initiated from a document column
+	copySrc string // Source path of document being copied
 
 	// Filter
 	filterInput   textinput.Model

pkg/cmd/browse/update.go

@@ -380,14 +380,13 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		m.statusMsg = status
 		m.statusMsgTime = time.Now()
 
-		// The new sibling lives in the collection column. When copying from a
-		// document column, that collection is one column to the left; the source
-		// still exists, so the column is kept (no removeLastColumn).
-		refreshIdx := m.activeColumn
-		if refreshIdx < len(m.columns) && m.columns[refreshIdx].isDoc {
-			refreshIdx--
-		}
-		if refreshIdx >= 0 && refreshIdx < len(m.columns) {
+		// Refresh the column showing the originating collection so the new
+		// sibling appears. The target is matched against the collection path
+		// captured in the message at copy time — never re-derived from
+		// m.activeColumn, which may have moved if the user navigated during the
+		// async copy. If that column is no longer present, refresh nothing.
+		refreshIdx := m.copyRefreshIndex(msg.collectionPath)
+		if refreshIdx >= 0 {
 			col := m.columns[refreshIdx]
 			sortField, sortDir := m.getSortParams(col.path)
 			m.loading = true
@@ -771,7 +770,6 @@ func (m Model) handleKeyPress(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 
 		if col.isDoc {
 			srcPath = col.path
-			m.copyFromDocView = true
 		} else {
 			item := m.getSelectedItem()
 			if item == nil || !item.isDoc {
@@ -780,7 +778,6 @@ func (m Model) handleKeyPress(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 				return m, clearStatusAfterDelay()
 			}
 			srcPath = item.path
-			m.copyFromDocView = false
 		}
 
 		m.copySrc = srcPath
@@ -938,10 +935,10 @@ func (m Model) handleOverlay(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 				dst = resolved
 			}
 
-			src, fromDocView := m.copySrc, m.copyFromDocView
+			src := m.copySrc
 			m.copySrc = ""
 			m.loading = true
-			return m, executeCopy(m.client, src, dst, fromDocView)
+			return m, executeCopy(m.client, src, dst)
 		case "esc":
 			m.overlay = OverlayNone
 			m.textInput.Blur()
@@ -1022,7 +1019,7 @@ func (m Model) handleOverlay(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 func (m Model) applySortAndClose() (tea.Model, tea.Cmd) {
 	// Get selected field (text input takes priority)
 	field := m.sortDialog.getSelectedField()
-	
+
 	if field == "" {
 		m.statusMsg = "No field selected"
 		m.statusMsgTime = time.Now()