From 4edcd5e9a5e4a0adae6de68f65b1f2b0901b577c Mon Sep 17 00:00:00 2001 From: Sriram-PR <160517347+Sriram-PR@users.noreply.github.com> Date: Mon, 5 Oct 2026 20:56:08 +1100 Subject: [PATCH 1/2] fix(runtime): read indexed servers before active ones in orphan cleanup Refs #1465 (item 2). cleanupOrphanedIndexEntries snapshotted the active server names before listing the indexed ones. When the startup pass ran late, a server registered and indexed between the two reads was deleted from the index as an orphan. That is what made TestRetrieveTools_ProfileWidenedByRawApplyIsSearchableImmediately return no hits on slow runners. Read the indexed names first: a server is registered before its tools are indexed, so one added mid-cleanup is either not indexed yet or already active. The comparison moves into findOrphanedIndexServers so the ordering is tested without relying on timing. --- internal/runtime/lifecycle.go | 61 +++++++++++++++++-------- internal/runtime/orphan_cleanup_test.go | 46 +++++++++++++++++++ 2 files changed, 87 insertions(+), 20 deletions(-) create mode 100644 internal/runtime/orphan_cleanup_test.go diff --git a/internal/runtime/lifecycle.go b/internal/runtime/lifecycle.go index 2970692c3..e95e84ee8 100644 --- a/internal/runtime/lifecycle.go +++ b/internal/runtime/lifecycle.go @@ -2434,36 +2434,29 @@ func (r *Runtime) cleanupOrphanedIndexEntries() { r.logger.Debug("Checking for orphaned index entries") - activeServers := r.upstreamManager.GetAllServerNames() - activeServerMap := make(map[string]bool) - for _, serverName := range activeServers { - activeServerMap[serverName] = true - } - - indexedServers, err := r.indexManager.GetAllIndexedServerNames() + orphans, activeCount, indexedCount, err := findOrphanedIndexServers( + r.indexManager.GetAllIndexedServerNames, r.upstreamManager.GetAllServerNames) if err != nil { r.logger.Warn("Failed to retrieve indexed server names for orphan cleanup", zap.Error(err)) return } var removedCount int - for _, indexedServer := range indexedServers { - if !activeServerMap[indexedServer] { - r.logger.Info("Removing orphaned index entries for server no longer in config", - zap.String("server", indexedServer)) - if err := r.indexManager.DeleteServerTools(indexedServer); err != nil { - r.logger.Warn("Failed to delete orphaned index entries", - zap.String("server", indexedServer), - zap.Error(err)) - } else { - removedCount++ - } + for _, indexedServer := range orphans { + r.logger.Info("Removing orphaned index entries for server no longer in config", + zap.String("server", indexedServer)) + if err := r.indexManager.DeleteServerTools(indexedServer); err != nil { + r.logger.Warn("Failed to delete orphaned index entries", + zap.String("server", indexedServer), + zap.Error(err)) + } else { + removedCount++ } } r.logger.Debug("Orphaned index cleanup completed", - zap.Int("active_servers", len(activeServers)), - zap.Int("indexed_servers", len(indexedServers)), + zap.Int("active_servers", activeCount), + zap.Int("indexed_servers", indexedCount), zap.Int("orphans_removed", removedCount)) if removedCount > 0 { @@ -2472,6 +2465,34 @@ func (r *Runtime) cleanupOrphanedIndexEntries() { } } +// findOrphanedIndexServers returns the indexed servers that are not active, +// plus the sizes of both lists for logging. +func findOrphanedIndexServers( + listIndexed func() ([]string, error), + listActive func() []string, +) (orphans []string, activeCount, indexedCount int, err error) { + // Indexed first: a server is registered before its tools are indexed, so a + // server added while this runs is either missing from the indexed list or + // already present in the active list read after it. + indexedServers, err := listIndexed() + if err != nil { + return nil, 0, 0, err + } + + activeServers := listActive() + active := make(map[string]bool, len(activeServers)) + for _, name := range activeServers { + active[name] = true + } + + for _, name := range indexedServers { + if !active[name] { + orphans = append(orphans, name) + } + } + return orphans, len(activeServers), len(indexedServers), nil +} + // supervisorEventForwarder subscribes to supervisor events and emits runtime events // to notify Web UI via SSE when server connection state changes. func (r *Runtime) supervisorEventForwarder() { diff --git a/internal/runtime/orphan_cleanup_test.go b/internal/runtime/orphan_cleanup_test.go new file mode 100644 index 000000000..9ee5a3608 --- /dev/null +++ b/internal/runtime/orphan_cleanup_test.go @@ -0,0 +1,46 @@ +package runtime + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A server registered and indexed while the startup orphan cleanup runs must +// not be treated as an orphan. The event fires right after the cleanup's first +// read, whichever list that is. +func TestFindOrphanedIndexServers_ServerAddedDuringCleanupIsKept(t *testing.T) { + var active, indexed []string + added := false + addServer := func() { + if !added { + added = true + active = append(active, "b") + indexed = append(indexed, "b") + } + } + listActive := func() []string { + defer addServer() + return append([]string(nil), active...) + } + listIndexed := func() ([]string, error) { + defer addServer() + return append([]string(nil), indexed...), nil + } + + orphans, _, _, err := findOrphanedIndexServers(listIndexed, listActive) + require.NoError(t, err) + assert.Empty(t, orphans) +} + +func TestFindOrphanedIndexServers_RemovedServerIsOrphan(t *testing.T) { + orphans, activeCount, indexedCount, err := findOrphanedIndexServers( + func() ([]string, error) { return []string{"a", "gone"}, nil }, + func() []string { return []string{"a"} }, + ) + require.NoError(t, err) + assert.Equal(t, []string{"gone"}, orphans) + assert.Equal(t, 1, activeCount) + assert.Equal(t, 2, indexedCount) +} From 8597fbb7f1dddf265d1f57987af18057c828f93b Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 6 Oct 2026 17:09:13 +0300 Subject: [PATCH 2/2] docs(runtime): state the limits of the indexed-first ordering in orphan cleanup --- internal/runtime/lifecycle.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/runtime/lifecycle.go b/internal/runtime/lifecycle.go index e95e84ee8..a205f2d47 100644 --- a/internal/runtime/lifecycle.go +++ b/internal/runtime/lifecycle.go @@ -2473,7 +2473,10 @@ func findOrphanedIndexServers( ) (orphans []string, activeCount, indexedCount int, err error) { // Indexed first: a server is registered before its tools are indexed, so a // server added while this runs is either missing from the indexed list or - // already present in the active list read after it. + // already present in the active list read after it. This covers servers newly + // added during cleanup; index entries persisted from a previous run can still + // be pruned if their server's async re-registration has not finished yet, and + // are re-indexed once discovery runs for it. indexedServers, err := listIndexed() if err != nil { return nil, 0, 0, err