fix(plugin-mcp): registerPluginMCPTools accepts registry + tests + remove pluginLookup

Author: Delqhi

cmd/sin-code/internal/plugins/registry.go

@@ -133,6 +133,7 @@ type MCPToolDef struct {
 	Name        string
 	Description string
 	Plugin      string
+	PluginPath  string
 	Tool        string
 	Binary      string
 	Args        []string
@@ -171,6 +172,7 @@ func (r *Registry) MCPTools() []MCPToolDef {
 				Name:        "sin_plugin_" + p.Name + "_" + t.Name,
 				Description: desc,
 				Plugin:      p.Name,
+				PluginPath:  p.Path,
 				Tool:        t.Name,
 				Binary:      t.Binary,
 				Args:        t.Args,

cmd/sin-code/internal/serve.go

@@ -602,6 +602,10 @@ func registerAllMCPTools(server *mcp.Server) {
 func registerPluginMCPTools(server *mcp.Server) {
 	reg := plugins.NewRegistry()
 	_ = reg.LoadFromDir("")
+	registerPluginMCPToolsWithReg(server, reg)
+}
+
+func registerPluginMCPToolsWithReg(server *mcp.Server, reg *plugins.Registry) {
 	for _, pt := range reg.MCPTools() {
 		pt := pt
 		server.AddTool(&mcp.Tool{
@@ -631,13 +635,9 @@ func registerPluginMCPTools(server *mcp.Server) {
 // path is resolved relative to the plugin dir; stdout/stderr are merged
 // and returned as a string. Timeout defaults to 60s.
 func runPluginMCPTool(ctx context.Context, pt plugins.MCPToolDef, args map[string]any) (string, error) {
-	plugin, ok := pluginLookup(pt.Plugin)
-	if !ok {
-		return "", fmt.Errorf("plugin %q not loaded", pt.Plugin)
-	}
 	fullPath := pt.Binary
 	if !filepath.IsAbs(fullPath) {
-		fullPath = filepath.Join(plugin.Path, fullPath)
+		fullPath = filepath.Join(pt.PluginPath, fullPath)
 	}
 	cmdArgs := make([]string, 0, len(pt.Args)+len(args))
 	for _, a := range pt.Args {
@@ -653,21 +653,15 @@ func runPluginMCPTool(ctx context.Context, pt plugins.MCPToolDef, args map[strin
 	execCtx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
 	c := exec.CommandContext(execCtx, fullPath, cmdArgs...)
-	c.Dir = plugin.Path
+	c.Dir = pt.PluginPath
 	c.Env = append(os.Environ(), "SIN_PLUGIN="+pt.Plugin, "SIN_PLUGIN_TOOL="+pt.Tool)
 	out, err := c.CombinedOutput()
 	if err != nil {
-		return fmt.Sprintf("%s\nERROR: %v", string(out), err), nil
+		return string(out), fmt.Errorf("plugin tool %q: %w", pt.Name, err)
 	}
 	return string(out), nil
 }
 
-func pluginLookup(name string) (*plugins.Plugin, bool) {
-	reg := plugins.NewRegistry()
-	_ = reg.LoadFromDir("")
-	return reg.Get(name)
-}
-
 func handleDiscover(ctx context.Context, args map[string]any) (string, error) {
 	// discover takes path as positional argument, not --path
 	path := "."

cmd/sin-code/internal/serve_test.go

@@ -3,7 +3,12 @@
 package internal
 
 import (
+	"context"
 	"testing"
+	"time"
+
+	"github.com/OpenSIN-Code/SIN-Code-Bundle/cmd/sin-code/internal/plugins"
+	"github.com/modelcontextprotocol/go-sdk/mcp"
 )
 
 func TestServeCmd_Flags(t *testing.T) {
@@ -34,3 +39,163 @@ func TestRegisterAllMCPTools(t *testing.T) {
 		t.Errorf("expected 13 tools, test config has %d", len(expectedTools))
 	}
 }
+
+func newPluginTestServer(t *testing.T, reg *plugins.Registry) *mcp.Server {
+	t.Helper()
+	server := mcp.NewServer(&mcp.Implementation{
+		Name:    "sin-code-plugin-test",
+		Version: "test",
+	}, &mcp.ServerOptions{
+		Capabilities: &mcp.ServerCapabilities{
+			Tools: &mcp.ToolCapabilities{},
+		},
+	})
+	registerAllMCPTools(server)
+	registerPluginMCPToolsWithReg(server, reg)
+	return server
+}
+
+func connectWithPluginReg(t *testing.T, reg *plugins.Registry) (*mcp.ClientSession, context.CancelFunc) {
+	t.Helper()
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	server := newPluginTestServer(t, reg)
+	cTransport, sTransport := mcp.NewInMemoryTransports()
+	ss, err := server.Connect(ctx, sTransport, nil)
+	if err != nil {
+		cancel()
+		t.Fatalf("server connect: %v", err)
+	}
+	t.Cleanup(func() { ss.Close() })
+	client := mcp.NewClient(&mcp.Implementation{
+		Name:    "test-client",
+		Version: "0.1.0",
+	}, nil)
+	cs, err := client.Connect(ctx, cTransport, nil)
+	if err != nil {
+		cancel()
+		ss.Close()
+		t.Fatalf("client connect: %v", err)
+	}
+	t.Cleanup(func() { cs.Close() })
+	return cs, cancel
+}
+
+func TestPluginToolRegistered(t *testing.T) {
+	reg := plugins.NewRegistry()
+	reg.Register(&plugins.Plugin{
+		Name:    "testplug",
+		Enabled: true,
+		Tools: []plugins.PluginTool{
+			{
+				Name:        "reverse",
+				Description: "Reverse a string",
+				Binary:      "/bin/echo",
+				Args:        []string{"input"},
+				Timeout:     10,
+			},
+		},
+	})
+	cs, cancel := connectWithPluginReg(t, reg)
+	defer cancel()
+
+	toolsResult, err := cs.ListTools(context.Background(), nil)
+	if err != nil {
+		t.Fatalf("ListTools failed: %v", err)
+	}
+
+	var names []string
+	for _, tool := range toolsResult.Tools {
+		names = append(names, tool.Name)
+	}
+
+	want := "sin_plugin_testplug_reverse"
+	found := false
+	for _, n := range names {
+		if n == want {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("expected plugin tool %q in list; got %v", want, names)
+	}
+}
+
+func TestPluginToolCallSuccess(t *testing.T) {
+	reg := plugins.NewRegistry()
+	reg.Register(&plugins.Plugin{
+		Name:    "echoplug",
+		Enabled: true,
+		Path:    "/tmp",
+		Tools: []plugins.PluginTool{
+			{
+				Name:        "say",
+				Description: "Echo back input",
+				Binary:      "/bin/echo",
+				Args:        []string{"input"},
+				Timeout:     10,
+			},
+		},
+	})
+	cs, cancel := connectWithPluginReg(t, reg)
+	defer cancel()
+
+	result, err := cs.CallTool(context.Background(), &mcp.CallToolParams{
+		Name:      "sin_plugin_echoplug_say",
+		Arguments: map[string]any{"input": "hello-world"},
+	})
+	if err != nil {
+		t.Fatalf("CallTool failed: %v", err)
+	}
+	if len(result.Content) == 0 {
+		t.Fatal("expected non-empty Content")
+	}
+	tc, ok := result.Content[0].(*mcp.TextContent)
+	if !ok {
+		t.Fatalf("expected TextContent, got %T", result.Content[0])
+	}
+	if tc.Text != "--input hello-world\n" {
+		t.Errorf("expected '--input hello-world\\n', got %q", tc.Text)
+	}
+}
+
+func TestPluginToolCallMissingBinary(t *testing.T) {
+	reg := plugins.NewRegistry()
+	reg.Register(&plugins.Plugin{
+		Name:    "badplug",
+		Enabled: true,
+		Path:    "/tmp",
+		Tools: []plugins.PluginTool{
+			{
+				Name:        "broken",
+				Description: "Intentionally missing binary",
+				Binary:      "./nonexistent-binary-xyz",
+				Args:        []string{},
+				Timeout:     5,
+			},
+		},
+	})
+	cs, cancel := connectWithPluginReg(t, reg)
+	defer cancel()
+
+	result, err := cs.CallTool(context.Background(), &mcp.CallToolParams{
+		Name:      "sin_plugin_badplug_broken",
+		Arguments: map[string]any{},
+	})
+	if err != nil {
+		t.Fatalf("CallTool failed (expected error in result, not call-level): %v", err)
+	}
+	if !result.IsError {
+		t.Error("expected IsError=true for missing binary, got false")
+	}
+	if len(result.Content) == 0 {
+		t.Fatal("expected non-empty Content")
+	}
+	tc, ok := result.Content[0].(*mcp.TextContent)
+	if !ok {
+		t.Fatalf("expected TextContent, got %T", result.Content[0])
+	}
+	if tc.Text == "" {
+		t.Errorf("expected error message in content, got empty")
+	}
+}

cmd/sin-code/testdata/scripts/plugin_mcp.txt

@@ -12,7 +12,7 @@ sin-code execute --command 'printf "name = \"mcp-test-plugin\"\nversion = \"1.0.
 stdout 'Exit:'
 
 # Plugin binary: reads --input arg and outputs reversed string
-sin-code execute --command 'printf "#!/bin/bash\nwhile [[ \$# -gt 0 ]]; do\n  case \$1 in\n    --input)\n      echo -n \"\$2\" | rev\n      shift 2\n      ;;\n    *)\n      shift\n      ;;\n  done\ndone\n" > $WORK/source-plugin/bin/reverse && chmod +x $WORK/source-plugin/bin/reverse'
+sin-code execute --command 'printf "#!/bin/bash\nwhile [[ \$# -gt 0 ]]; do\n  case \$1 in\n    --input)\n      echo -n \"\$2\" | rev\n      shift 2\n      ;;\n    *)\n      shift\n      ;;\n  esac\ndone\n" > $WORK/source-plugin/bin/reverse && chmod +x $WORK/source-plugin/bin/reverse'
 stdout 'Exit:'
 
 # Install plugin