Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cmd/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,9 @@ func parseServerEntry(pkg packages.Server, opts serverEntryOptions) (config.Serv
if err != nil {
return config.ServerEntry{}, fmt.Errorf("error matching requested tools: %w", err)
}
if len(requestedTools) == 0 {
return config.ServerEntry{}, fmt.Errorf("tools not available")
}

selectedRuntime, err := selectRuntime(pkg.Installations, opts.Runtime, opts.SupportedRuntimes)
if err != nil {
Expand Down
34 changes: 34 additions & 0 deletions cmd/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,40 @@ func TestParseServerEntry(t *testing.T) {
isErrorExpected: true,
expectedErrorMessage: "installation server name is missing for runtime 'docker'",
},
{
name: "server with no tools available",
installations: map[runtime.Runtime]packages.Installation{
runtime.UVX: {
Package: "mcp-server-no-tools",
Recommended: true,
},
},
supportedRuntimes: []runtime.Runtime{runtime.UVX},
pkgName: "no-tools",
pkgID: "no-tools",
availableTools: []string{}, // No tools available
requestedTools: []string{},
arguments: packages.Arguments{},
isErrorExpected: true,
expectedErrorMessage: "tools not available",
},
{
name: "server with requested tools not available",
installations: map[runtime.Runtime]packages.Installation{
runtime.UVX: {
Package: "mcp-server-limited",
Recommended: true,
},
},
supportedRuntimes: []runtime.Runtime{runtime.UVX},
pkgName: "limited",
pkgID: "limited",
availableTools: []string{"toolA", "toolB"},
requestedTools: []string{"toolC", "toolD"}, // Requesting unavailable tools
arguments: packages.Arguments{},
isErrorExpected: true,
expectedErrorMessage: "error matching requested tools: none of the requested values were found",
},
{
name: "requested tool not found",
installations: map[runtime.Runtime]packages.Installation{
Expand Down
6 changes: 6 additions & 0 deletions cmd/config/tools/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ func (c *RemoveCmd) run(cmd *cobra.Command, args []string) error {
}
}

// Validate that at least one tool remains.
if len(srv.Tools) == 0 {
return fmt.Errorf("cannot remove all tools from server '%s'\n"+
"To remove the server instead use: mcpd remove %s", serverName, serverName)
}

// Update server in config by removing and re-adding.
err = cfg.RemoveServer(serverName)
if err != nil {
Expand Down
128 changes: 128 additions & 0 deletions cmd/config/tools/remove_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package tools

import (
"bytes"
"testing"

"github.com/stretchr/testify/require"

"github.com/mozilla-ai/mcpd/v2/internal/cmd"
cmdopts "github.com/mozilla-ai/mcpd/v2/internal/cmd/options"
"github.com/mozilla-ai/mcpd/v2/internal/config"
)

type mockConfigForRemove struct {
servers []config.ServerEntry
addErr error
removeErr error
}

func (m *mockConfigForRemove) AddServer(entry config.ServerEntry) error {
if m.addErr != nil {
return m.addErr
}
m.servers = append(m.servers, entry)
return nil
}

func (m *mockConfigForRemove) RemoveServer(name string) error {
if m.removeErr != nil {
return m.removeErr
}
for i, s := range m.servers {
if s.Name == name {
m.servers = append(m.servers[:i], m.servers[i+1:]...)
return nil
}
}
return nil
}

func (m *mockConfigForRemove) ListServers() []config.ServerEntry {
return m.servers
}

func (m *mockConfigForRemove) SaveConfig() error {
return nil
}

type mockLoaderForRemove struct {
cfg *mockConfigForRemove
err error
}

func (m *mockLoaderForRemove) Load(_ string) (config.Modifier, error) {
return m.cfg, m.err
}

func TestRemoveCmd_RemoveAllTools(t *testing.T) {
t.Parallel()

tests := []struct {
name string
serverName string
initialTools []string
toolsToRemove []string
expectedError string
}{
{
name: "removing all tools should fail",
serverName: "test-server",
initialTools: []string{"tool1", "tool2"},
toolsToRemove: []string{"tool1", "tool2"},
expectedError: "cannot remove all tools from server 'test-server'\nTo remove the server instead use: mcpd remove test-server",
},
{
name: "removing last tool should fail",
serverName: "test-server",
initialTools: []string{"tool1"},
toolsToRemove: []string{"tool1"},
expectedError: "cannot remove all tools from server 'test-server'\nTo remove the server instead use: mcpd remove test-server",
},
{
name: "removing some tools should succeed",
serverName: "test-server",
initialTools: []string{"tool1", "tool2", "tool3"},
toolsToRemove: []string{"tool1", "tool2"},
expectedError: "",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

cfg := &mockConfigForRemove{
servers: []config.ServerEntry{
{
Name: tc.serverName,
Tools: tc.initialTools,
},
},
}

loader := &mockLoaderForRemove{cfg: cfg}
baseCmd := &cmd.BaseCmd{}

removeCmd, err := NewRemoveCmd(baseCmd, cmdopts.WithConfigLoader(loader))
require.NoError(t, err)

// Prepare arguments: server-name followed by tools to remove.
args := append([]string{tc.serverName}, tc.toolsToRemove...)

var out bytes.Buffer
removeCmd.SetOut(&out)
removeCmd.SetErr(&out)

err = removeCmd.RunE(removeCmd, args)

if tc.expectedError != "" {
require.EqualError(t, err, tc.expectedError)
} else {
require.NoError(t, err)
// Verify some tools remain.
require.Greater(t, len(cfg.servers[0].Tools), 0)
}
})
}
}
1 change: 1 addition & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ The reload process maintains strict consistency - any error causes the daemon to

- **Configuration errors**: Invalid configuration files or loading failures cause the daemon to exit
- **Validation errors**: Invalid server configurations cause the daemon to exit
- **No tools configured**: If a server configuration has no tools (empty tools list or manually removed from config), the daemon will exit with an error
- **Server operation failures**: Any failure to start, stop, or restart a server causes the daemon to exit

This ensures the daemon never runs in an inconsistent or partially-failed state, matching the behavior during initial startup where any server failure prevents the daemon from running.
Expand Down
10 changes: 10 additions & 0 deletions internal/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,17 @@ func (d *Daemon) startMCPServers(ctx context.Context) error {
return nil
}

// startMCPServer starts a single MCP server and registers it with the daemon.
// It validates that the server has tools and a supported runtime before initializing.
func (d *Daemon) startMCPServer(ctx context.Context, server runtime.Server) error {
// Validate that the server has tools configured.
if len(server.Tools) == 0 {
return fmt.Errorf(
"server '%s' has no tools configured - MCP servers require at least one tool to function",
server.Name(),
)
}

runtimeBinary := server.Runtime()
if _, supported := d.supportedRuntimes[runtime.Runtime(runtimeBinary)]; !supported {
return fmt.Errorf(
Expand Down
61 changes: 61 additions & 0 deletions internal/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,67 @@ func TestDaemon_CloseClientWithTimeout_Direct(t *testing.T) {
}
}

// TestDaemon_StartMCPServer_NoTools tests that servers without tools are rejected
func TestDaemon_StartMCPServer_NoTools(t *testing.T) {
t.Parallel()

tests := []struct {
name string
serverName string
tools []string
expectedError string
}{
{
name: "server with no tools should fail",
serverName: "test-no-tools",
tools: []string{},
expectedError: "server 'test-no-tools' has no tools configured - MCP servers require at least one tool to function",
},
{
name: "server with empty tools list should fail",
serverName: "test-empty-tools",
tools: []string{},
expectedError: "server 'test-empty-tools' has no tools configured - MCP servers require at least one tool to function",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
t.Cleanup(cancel)

logger := hclog.NewNullLogger()

// Create a daemon with a dummy server to satisfy validation.
dummyServer := runtime.Server{
ServerEntry: config.ServerEntry{
Name: "dummy",
Package: "uvx::dummy",
},
}
deps, err := NewDependencies(logger, ":8085", []runtime.Server{dummyServer})
require.NoError(t, err)
daemon, err := NewDaemon(deps)
require.NoError(t, err)

// Create server with specified tools.
server := runtime.Server{
ServerEntry: config.ServerEntry{
Name: tc.serverName,
Package: "uvx::test-package",
Tools: tc.tools,
},
ServerExecutionContext: configcontext.ServerExecutionContext{},
}

err = daemon.startMCPServer(ctx, server)
require.EqualError(t, err, tc.expectedError)
})
}
}

// TestDaemon_CloseClientWithTimeout_ReturnValue tests the return value behavior of closeClientWithTimeout
func TestDaemon_CloseClientWithTimeout_ReturnValue(t *testing.T) {
t.Parallel()
Expand Down