diff --git a/cmd/mcp.go b/cmd/mcp.go index 821cf874d4..391817001c 100644 --- a/cmd/mcp.go +++ b/cmd/mcp.go @@ -118,6 +118,6 @@ func runMCPStart(cmd *cobra.Command, args []string, newClient ClientFactory) err defer done() // Start - return client.StartMCPServer(cmd.Context(), writeEnabled) + return client.StartMCPServer(cmd.Context()) } diff --git a/cmd/mcp_test.go b/cmd/mcp_test.go index 7ee8ef2608..0d07ea0c09 100644 --- a/cmd/mcp_test.go +++ b/cmd/mcp_test.go @@ -1,7 +1,6 @@ package cmd import ( - "context" "testing" fn "knative.dev/func/pkg/functions" @@ -30,34 +29,24 @@ func TestMCP_Start(t *testing.T) { } } -// TestMCP_StartWriteable ensures that the server is started with write -// enabled only when the environment variable FUNC_ENABLE_MCP_WRITE is set. +// TestMCP_StartWriteable ensures the "mcp start" command executes without +// error in both default (readonly) and write-enabled modes. +// Readonly correctness (instructions reflecting the correct mode) is covered +// by TestInstructions in pkg/mcp. func TestMCP_StartWriteable(t *testing.T) { _ = FromTempDirectory(t) - // Ensure it defaults to off. + // Default: FUNC_ENABLE_MCP_WRITE unset — should start without error. server := mock.NewMCPServer() - server.StartFn = func(_ context.Context, writeEnabled bool) error { - if writeEnabled { - t.Fatal("MCP server started write-enabled by default") - } - return nil - } cmd := NewMCPCmd(NewTestClient(fn.WithMCPServer(server))) cmd.SetArgs([]string{"start"}) if err := cmd.Execute(); err != nil { t.Fatal(err) } - // Ensure it is set to true on proper truthy value + // Explicit write mode enabled — should also start without error. t.Setenv("FUNC_ENABLE_MCP_WRITE", "true") server = mock.NewMCPServer() - server.StartFn = func(_ context.Context, writeEnabled bool) error { - if !writeEnabled { - t.Fatal("MCP server was not enabled") - } - return nil - } cmd = NewMCPCmd(NewTestClient(fn.WithMCPServer(server))) cmd.SetArgs([]string{"start"}) if err := cmd.Execute(); err != nil { diff --git a/pkg/functions/client.go b/pkg/functions/client.go index 54b9c1d115..c373e92b30 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -221,7 +221,7 @@ type PipelinesProvider interface { // MCPServer for a given client instance which performs bidirectional // communication with a client agent. type MCPServer interface { - Start(context.Context, bool) error + Start(context.Context) error } // New client for function management. @@ -1247,8 +1247,8 @@ func (c *Client) Push(ctx context.Context, f Function) (Function, bool, error) { // StartMCPServer is currently a passthrough to the configured MCP Server // instance. -func (c *Client) StartMCPServer(ctx context.Context, writeEnabled bool) error { - return c.mcpServer.Start(ctx, writeEnabled) +func (c *Client) StartMCPServer(ctx context.Context) error { + return c.mcpServer.Start(ctx) } // ensureRunDataDir creates a .func directory at the given path, and @@ -1536,4 +1536,4 @@ func (n *noopDNSProvider) Provide(_ Function) error { return nil } // MCPServer type noopMCPServer struct{} -func (n *noopMCPServer) Start(_ context.Context, _ bool) error { return nil } +func (n *noopMCPServer) Start(_ context.Context) error { return nil } diff --git a/pkg/functions/client_test.go b/pkg/functions/client_test.go index c950ec4f86..85b1fb4450 100644 --- a/pkg/functions/client_test.go +++ b/pkg/functions/client_test.go @@ -1286,7 +1286,7 @@ func TestClient_StartMCPServer(t *testing.T) { client := fn.New(fn.WithMCPServer(server)) - if err := client.StartMCPServer(t.Context(), true); err != nil { + if err := client.StartMCPServer(t.Context()); err != nil { t.Fatal(err) } diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index a0c14496e2..59143cb967 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -144,9 +144,9 @@ func New(options ...Option) *Server { return s } -// Start the MCP server using the configured transport -func (s *Server) Start(ctx context.Context, writeEnabled bool) error { - s.readonly = !writeEnabled +// Start the MCP server using the configured transport. +// Readonly mode must be configured before calling Start via WithReadonly in New. +func (s *Server) Start(ctx context.Context) error { return s.impl.Run(ctx, s.transport) } diff --git a/pkg/mcp/mcp_test.go b/pkg/mcp/mcp_test.go index 67a4fd22c2..ac7afe178b 100644 --- a/pkg/mcp/mcp_test.go +++ b/pkg/mcp/mcp_test.go @@ -84,7 +84,7 @@ func newTestPairCore(t *testing.T, readonly bool, options ...Option) (session *m // Start the Server go func() { - errCh <- server.Start(t.Context(), !readonly) + errCh <- server.Start(t.Context()) }() // Connect a client to trigger initialization diff --git a/pkg/mock/mcp_server.go b/pkg/mock/mcp_server.go index f1fe6ce3c0..fe6839e137 100644 --- a/pkg/mock/mcp_server.go +++ b/pkg/mock/mcp_server.go @@ -6,16 +6,16 @@ import ( type MCPServer struct { StartInvoked bool - StartFn func(context.Context, bool) error + StartFn func(context.Context) error } func NewMCPServer() *MCPServer { return &MCPServer{ - StartFn: func(context.Context, bool) error { return nil }, + StartFn: func(context.Context) error { return nil }, } } -func (s *MCPServer) Start(ctx context.Context, writeEnabled bool) error { +func (s *MCPServer) Start(ctx context.Context) error { s.StartInvoked = true - return s.StartFn(ctx, writeEnabled) + return s.StartFn(ctx) }