From 2a4e8b7ba376884a997d63632d5df5c58cf6c04f Mon Sep 17 00:00:00 2001
From: Virgil <virgil@lethean.io>
Date: Thu, 2 Apr 2026 14:10:53 +0000
Subject: [PATCH] fix(mcp): snapshot exposed service slices

Return copies from service accessors and ignore nil subsystems during construction to keep the MCP service API stable and AX-friendly.

Co-Authored-By: Virgil <virgil@lethean.io>
---
 pkg/mcp/mcp.go            | 16 ++++++++++------
 pkg/mcp/subsystem_test.go | 21 +++++++++++++++++++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go
index 9e340a9..b5ae8a9 100644
--- a/pkg/mcp/mcp.go
+++ b/pkg/mcp/mcp.go
@@ -83,7 +83,6 @@ func New(opts Options) (*Service, error) {
 		server:         server,
 		processService: opts.ProcessService,
 		wsHub:          opts.WSHub,
-		subsystems:     opts.Subsystems,
 		logger:         log.Default(),
 		processMeta:    make(map[string]processRuntime),
 	}
@@ -115,7 +114,12 @@ func New(opts Options) (*Service, error) {
 
 	s.registerTools(s.server)
 
-	for _, sub := range s.subsystems {
+	s.subsystems = make([]Subsystem, 0, len(opts.Subsystems))
+	for _, sub := range opts.Subsystems {
+		if sub == nil {
+			continue
+		}
+		s.subsystems = append(s.subsystems, sub)
 		if sn, ok := sub.(SubsystemWithNotifier); ok {
 			sn.SetNotifier(s)
 		}
@@ -141,7 +145,7 @@ func New(opts Options) (*Service, error) {
 //	    fmt.Println(sub.Name())
 //	}
 func (s *Service) Subsystems() []Subsystem {
-	return s.subsystems
+	return slices.Clone(s.subsystems)
 }
 
 // SubsystemsSeq returns an iterator over the registered subsystems.
@@ -150,7 +154,7 @@ func (s *Service) Subsystems() []Subsystem {
 //	    fmt.Println(sub.Name())
 //	}
 func (s *Service) SubsystemsSeq() iter.Seq[Subsystem] {
-	return slices.Values(s.subsystems)
+	return slices.Values(slices.Clone(s.subsystems))
 }
 
 // Tools returns all recorded tool metadata.
@@ -159,7 +163,7 @@ func (s *Service) SubsystemsSeq() iter.Seq[Subsystem] {
 //	    fmt.Printf("%s (%s): %s\n", t.Name, t.Group, t.Description)
 //	}
 func (s *Service) Tools() []ToolRecord {
-	return s.tools
+	return slices.Clone(s.tools)
 }
 
 // ToolsSeq returns an iterator over all recorded tool metadata.
@@ -168,7 +172,7 @@ func (s *Service) Tools() []ToolRecord {
 //	    fmt.Println(rec.Name)
 //	}
 func (s *Service) ToolsSeq() iter.Seq[ToolRecord] {
-	return slices.Values(s.tools)
+	return slices.Values(slices.Clone(s.tools))
 }
 
 // Shutdown gracefully shuts down all subsystems that support it.
diff --git a/pkg/mcp/subsystem_test.go b/pkg/mcp/subsystem_test.go
index bf07a87..92bf0e9 100644
--- a/pkg/mcp/subsystem_test.go
+++ b/pkg/mcp/subsystem_test.go
@@ -88,6 +88,27 @@ func TestSubsystem_Good_MultipleSubsystems(t *testing.T) {
 	}
 }
 
+func TestSubsystem_Good_NilEntriesIgnoredAndSnapshots(t *testing.T) {
+	sub := &stubSubsystem{name: "snap-sub"}
+	svc, err := New(Options{Subsystems: []Subsystem{nil, sub}})
+	if err != nil {
+		t.Fatalf("New() failed: %v", err)
+	}
+
+	subs := svc.Subsystems()
+	if len(subs) != 1 {
+		t.Fatalf("expected 1 subsystem after filtering nil entries, got %d", len(subs))
+	}
+	if subs[0].Name() != "snap-sub" {
+		t.Fatalf("expected snap-sub, got %q", subs[0].Name())
+	}
+
+	subs[0] = nil
+	if svc.Subsystems()[0] == nil {
+		t.Fatal("expected Subsystems() to return a snapshot, not the live slice")
+	}
+}
+
 func TestSubsystem_Good_NotifierSetBeforeRegistration(t *testing.T) {
 	sub := &notifierSubsystem{stubSubsystem: stubSubsystem{name: "notifier-sub"}}
 	_, err := New(Options{Subsystems: []Subsystem{sub}})