Forget a peer's manual entry when the TUI removes it from the cluster

docs/terminal-interface.mdx

@@ -245,7 +245,7 @@ the rest are declined and their senders are told.
 | `d` | Decline an inbound pairing request |
 | `f` | Find a machine by address, without pairing with it |
 | `c` | Cancel the pairing request you sent |
-| `r` | Remove the selected machine from the cluster (asks you to confirm). For a machine you added with `f`, forget it straight away |
+| `r` | Remove the selected machine from the cluster (asks you to confirm), forgetting it too if you added it with `f`. A machine you added with `f` that is not a member is forgotten straight away |
 | `l` | Leave the cluster (asks you to confirm) |
 | `/` | Filter the list by name or address; `esc` clears it |
 

services/nvpair-tui/ui/nodes.go

@@ -4,9 +4,11 @@
 package ui
 
 import (
+	"context"
 	"fmt"
 	"log/slog"
 	"net"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -1261,7 +1263,8 @@ func (v *nodesView) declineWaiting() []tea.Cmd {
 }
 
 // removeSelected drops the selected node's strongest relationship: cluster
-// membership if it is a member, otherwise the manual entry that added it.
+// membership if it is a member, along with the manual entry that added it,
+// otherwise that manual entry alone.
 func (v *nodesView) removeSelected() tea.Cmd {
 	row := v.selectedRow()
 	if row == nil {
@@ -1282,11 +1285,11 @@ func (v *nodesView) removeSelected() tea.Cmd {
 			row.name)
 		return nil
 
-	case row.manualID != "":
-		return call(v.client, "node/remove", map[string]string{"id": row.manualID},
-			func(_ *rpc.Message, err error) tea.Msg {
-				return nodeActionMsg{what: "remove manual entry " + row.name, err: err}
-			})
+	case len(row.manualIDs) > 0:
+		name, ids := row.name, row.manualIDs
+		return func() tea.Msg {
+			return nodeActionMsg{what: "remove manual entry " + name, err: forgetManualEntries(v.client, ids)}
+		}
 
 	default:
 		v.status.info("%s is only discovered - nothing to remove", row.name)
@@ -1358,17 +1361,23 @@ func (v *nodesView) handleInviteCancelled(msg inviteCancelledMsg) tea.Cmd {
 	return nil
 }
 
-// removeMember un-pairs a confirmed peer.
+// removeMember un-pairs a confirmed peer, then forgets every manual entry that
+// added it.
 //
 // Keyed by the stable nodeUuid rather than the display name: a member that
 // renamed its PC keeps its UUID, so matching on a possibly stale name would
 // silently fail. The row is looked up again by key so a list that re-sorted
 // between arming and confirming cannot redirect the removal at another node.
+//
+// The broker keeps manual entries on disk and re-adds them whenever it starts,
+// so one left behind brings the removed peer back as a manual row, here and in
+// the desktop app. They are forgotten only once the un-pairing succeeds: a peer
+// still in the cluster keeps the addresses that reach it.
 func (v *nodesView) removeMember(key string) tea.Cmd {
-	name := key
+	name, manualIDs := key, []string(nil)
 	for _, n := range v.all {
 		if n.key == key {
-			name = n.name
+			name, manualIDs = n.name, n.manualIDs
 			break
 		}
 	}
@@ -1377,10 +1386,31 @@ func (v *nodesView) removeMember(key string) tea.Cmd {
 	v.status.busy("removing %s from the cluster...", name)
 	return call(v.client, "nodes:remove", map[string]string{"nodeUuid": key},
 		func(_ *rpc.Message, err error) tea.Msg {
-			return nodeActionMsg{what: "remove " + name, err: err}
+			if err != nil {
+				return nodeActionMsg{what: "remove " + name, err: err}
+			}
+			// Still on the command's goroutine, so the follow-ups run in line.
+			if err := forgetManualEntries(v.client, manualIDs); err != nil {
+				return nodeActionMsg{what: "remove manual entry " + name, err: err}
+			}
+			return nodeActionMsg{what: "remove " + name}
 		})
 }
 
+// forgetManualEntries removes each manual entry in turn, stopping at the first
+// that fails. Each request gets its own callTimeout, as it would through call.
+func forgetManualEntries(client *rpc.Client, ids []string) error {
+	for _, id := range ids {
+		ctx, cancel := context.WithTimeout(context.Background(), callTimeout)
+		_, err := client.Call(ctx, "node/remove", map[string]string{"id": id})
+		cancel()
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // leaveCluster unjoins this node. The cluster-manager tears down local trust and
 // pushes cluster:identity-changed and nodes:changed, which refresh the view.
 func (v *nodesView) leaveCluster() tea.Cmd {
@@ -1444,9 +1474,9 @@ func (v *nodesView) followDetail() {
 			return
 		}
 	}
-	if id := v.detail.node.manualID; id != "" {
+	if ids := v.detail.node.manualIDs; len(ids) > 0 {
 		for _, n := range v.all {
-			if n.manualID == id {
+			if slices.Contains(n.manualIDs, ids[0]) {
 				v.detail.followNode(n, true)
 				return
 			}

services/nvpair-tui/ui/nodes_test.go

@@ -0,0 +1,147 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package ui
+
+import (
+	"context"
+	"encoding/json"
+	"net"
+	"testing"
+	"time"
+
+	"nvpair-tui/rpc"
+
+	tea "github.com/charmbracelet/bubbletea"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// removalCall is one request removalBroker received.
+type removalCall struct {
+	method string
+	params json.RawMessage
+}
+
+// removalBroker is a broker that records every call and answers it, failing
+// each call to the method named by fail.
+func removalBroker(t *testing.T, fail string) (*rpc.Client, <-chan removalCall) {
+	t.Helper()
+	c1, c2 := net.Pipe()
+	ctx, cancel := context.WithCancel(context.Background())
+	t.Cleanup(func() {
+		cancel()
+		c1.Close()
+		c2.Close()
+	})
+	client := rpc.NewClient(c1, c1)
+	go client.Run(ctx)
+
+	calls := make(chan removalCall, 8)
+	broker := rpc.NewCodec(c2, c2)
+	go func() {
+		for {
+			req, err := broker.Read()
+			if err != nil {
+				return
+			}
+			calls <- removalCall{method: req.Method, params: req.Params}
+			reply := &rpc.Message{JSONRPC: "2.0", ID: req.ID, Result: json.RawMessage(`{}`)}
+			if req.Method == fail {
+				reply = &rpc.Message{JSONRPC: "2.0", ID: req.ID,
+					Error: &rpc.RPCError{Code: -32000, Message: "refused"}}
+			}
+			_ = broker.Write(reply)
+		}
+	}()
+	return client, calls
+}
+
+// confirmRemoval arms and confirms removing the selected node, then feeds each
+// reply back into the view until it reports an outcome.
+func confirmRemoval(t *testing.T, v *nodesView) {
+	t.Helper()
+	require.Nil(t, v.removeSelected(), "removal was dispatched without confirmation")
+	cmd := v.handleKey(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("y")})
+	for cmd != nil {
+		msg := cmd()
+		if _, done := msg.(nodeActionMsg); done {
+			v.Update(msg)
+			return
+		}
+		cmd = v.Update(msg)
+	}
+	require.FailNow(t, "the removal ended without reporting an outcome")
+}
+
+// wantCall checks the next request the broker received carries field = value.
+func wantCall(t *testing.T, calls <-chan removalCall, method, field, value string) {
+	t.Helper()
+	select {
+	case c := <-calls:
+		require.Equal(t, method, c.method)
+		var params map[string]string
+		require.NoError(t, json.Unmarshal(c.params, &params), "decode %s params", method)
+		assert.Equal(t, value, params[field], "%s %s", method, field)
+	case <-time.After(2 * time.Second):
+		require.FailNow(t, "broker never received "+method)
+	}
+}
+
+// peerAddedByAddress is a nodes view with one member that was first added by
+// address, selected.
+func peerAddedByAddress(client *rpc.Client) *nodesView {
+	v := newNodesView(client)
+	v.feeds.discovered = []availableNode{{
+		HostUUID: "peer", Name: "peer", IPAddress: "10.0.0.2", Trusted: true,
+	}}
+	v.feeds.manual = []manualNode{{ID: "manual:10.0.0.2", Address: "10.0.0.2", HostUUID: "peer"}}
+	v.rebuild()
+	v.selectedKey = "peer"
+	return v
+}
+
+// TestRemoveMember_ForgetsItsManualEntry checks un-pairing a peer that was added
+// by address forgets that address too. The broker keeps manual entries on disk
+// and re-adds them when it starts, so one left behind brought the peer back.
+func TestRemoveMember_ForgetsItsManualEntry(t *testing.T) {
+	client, calls := removalBroker(t, "")
+	v := peerAddedByAddress(client)
+
+	confirmRemoval(t, v)
+
+	wantCall(t, calls, "nodes:remove", "nodeUuid", "peer")
+	wantCall(t, calls, "node/remove", "id", "manual:10.0.0.2")
+	assert.Contains(t, v.status.render(), "remove peer ok", "the removal is reported")
+}
+
+// TestRemoveMember_ForgetsEveryManualEntry checks a peer added twice, by
+// hostname and by IP, has both entries forgotten when it is un-paired.
+func TestRemoveMember_ForgetsEveryManualEntry(t *testing.T) {
+	client, calls := removalBroker(t, "")
+	v := peerAddedByAddress(client)
+	v.feeds.manual = append(v.feeds.manual,
+		manualNode{ID: "manual:peer.lan", Address: "peer.lan", HostUUID: "peer"})
+	v.rebuild()
+
+	confirmRemoval(t, v)
+
+	wantCall(t, calls, "nodes:remove", "nodeUuid", "peer")
+	wantCall(t, calls, "node/remove", "id", "manual:10.0.0.2")
+	wantCall(t, calls, "node/remove", "id", "manual:peer.lan")
+	assert.Contains(t, v.status.render(), "remove peer ok", "the removal is reported")
+}
+
+// TestRemoveMember_KeepsTheManualEntryWhenUnpairingFails checks a peer the cluster manager
+// would not un-pair keeps the address it was added by: it is still a member,
+// and that address is how it is reached.
+func TestRemoveMember_KeepsTheManualEntryWhenUnpairingFails(t *testing.T) {
+	client, calls := removalBroker(t, "nodes:remove")
+	v := peerAddedByAddress(client)
+
+	confirmRemoval(t, v)
+
+	wantCall(t, calls, "nodes:remove", "nodeUuid", "peer")
+	assert.Empty(t, calls, "a failed un-pairing must not go on to forget the manual entry")
+	assert.Contains(t, v.status.render(), "remove peer failed", "the failure is reported")
+}

services/nvpair-tui/ui/nodesmodel.go

@@ -113,9 +113,10 @@ type nodeRow struct {
 	membership nodeMembership
 	self       bool
 
-	// manualID is the handle node/remove needs. Empty unless the entry was
-	// added by hand.
-	manualID string
+	// manualIDs are the handles node/remove needs, one per manual entry that
+	// merged into this row: the same machine can be added twice, once by
+	// hostname and once by IP. Empty unless the node was added by hand.
+	manualIDs []string
 	// nodeInfoTLS is a hand-added node that serves node-info over TLS. That
 	// endpoint needs the backend's cluster trust, which this client does not
 	// hold, so it is never polled from here: probedTelemetry, the manual-node
@@ -213,7 +214,7 @@ func mergeNodes(in nodeFeeds) []nodeRow {
 			row.name = m.Name
 			row.address = m.Address
 		}
-		row.manualID = m.ID
+		row.manualIDs = append(row.manualIDs, m.ID)
 		if row.name == "" {
 			row.name = m.Address
 		}

services/nvpair-tui/ui/nodesmodel_test.go

@@ -152,7 +152,7 @@ func TestMerge_DeduplicatesAcrossFeeds(t *testing.T) {
 
 	require.Len(t, rows, 1, "one machine must produce one row")
 	row := rows[0]
-	assert.Equal(t, "m1", row.manualID, "manual handle must survive the merge")
+	assert.Equal(t, []string{"m1"}, row.manualIDs, "manual handle must survive the merge")
 	assert.Equal(t, nodeMembership(membershipMember), row.membership)
 	assert.Equal(t, 2, row.modelCount())
 }
@@ -191,7 +191,7 @@ func TestMerge_UsesEveryFactTheManualWorkerReports(t *testing.T) {
 		manual:     []manualNode{{ID: "m1", Address: "gpu-box.lan", HostUUID: "u1", NodeInfoUp: true}},
 	})
 	require.Len(t, rows, 1, "the manual entry must be joined by its UUID")
-	assert.Equal(t, "m1", rows[0].manualID, "the joined row must retain its manual handle")
+	assert.Equal(t, []string{"m1"}, rows[0].manualIDs, "the joined row must retain its manual handle")
 
 	// A hand-added node reached on a non-default node-info port, over TLS.
 	rows = mergeNodes(nodeFeeds{
@@ -209,6 +209,21 @@ func TestMerge_UsesEveryFactTheManualWorkerReports(t *testing.T) {
 	}
 }
 
+// TestMerge_KeepsEveryManualEntryForOneMachine checks a machine added twice, by
+// hostname and by IP, keeps both entries. Removing it must forget both, or the
+// one left behind brings it back when the broker next starts.
+func TestMerge_KeepsEveryManualEntryForOneMachine(t *testing.T) {
+	rows := mergeNodes(nodeFeeds{
+		discovered: []availableNode{{HostUUID: "u1", Name: "gpu-box", IPAddress: "10.0.0.5", Port: 14318}},
+		manual: []manualNode{
+			{ID: "m1", Address: "gpu-box.lan", HostUUID: "u1", NodeInfoUp: true},
+			{ID: "m2", Address: "10.0.0.5", HostUUID: "u1", NodeInfoUp: true},
+		},
+	})
+	require.Len(t, rows, 1, "both entries must join the one machine")
+	assert.Equal(t, []string{"m1", "m2"}, rows[0].manualIDs)
+}
+
 // TestAReusedAddressDoesNotMergeTwoMachines checks an address is not taken for
 // an identity. A manual entry whose probe read one UUID must not fold into a
 // discovered row with a different UUID at the same address — that is another
@@ -222,14 +237,14 @@ func TestAReusedAddress_DoesNotMergeTwoMachines(t *testing.T) {
 		manual:     []manualNode{{ID: "m1", Name: "a", Address: "10.0.0.5", HostUUID: "machine-a", NodeInfoUp: true}},
 	})
 	require.Len(t, rows, 2, "two machines at one address must remain distinct")
-	assert.Empty(t, findRow(t, rows, "b").manualID, "machine b must not take the manual entry for machine a")
+	assert.Empty(t, findRow(t, rows, "b").manualIDs, "machine b must not take the manual entry for machine a")
 
 	rows = mergeNodes(nodeFeeds{
 		discovered: discovered,
 		manual:     []manualNode{{ID: "m1", Address: "10.0.0.5"}},
 	})
 	require.Len(t, rows, 1, "an unidentified entry must join the machine at its address")
-	assert.Equal(t, "m1", rows[0].manualID)
+	assert.Equal(t, []string{"m1"}, rows[0].manualIDs)
 }
 
 // TestTLSNodeInfoIsNotPolledInPlainText checks the detail screen shows the