mcp: allow requests before notifications/initialized
Upon a closer reading of the spec, it is OK for the server to response to client requests before it has received notifications/initialized (as long as it has received initialize). This effectively rolls back the fix from #225. Some hooks are left for enforcing strictness around server->client requests prior to initialized. These will be revisited in subsequent CLs. For #395
This commit is contained in:
parent
b774809086
commit
5ccd97fafc
4 changed files with 53 additions and 6 deletions
|
|
@ -1415,7 +1415,7 @@ func TestElicitationCapabilityDeclaration(t *testing.T) {
|
||||||
RequestedSchema: &jsonschema.Schema{Type: "object"},
|
RequestedSchema: &jsonschema.Schema{Type: "object"},
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Errorf("elicitation should work when capability is declared, got error: %v", err)
|
t.Fatalf("elicitation should work when capability is declared, got error: %v", err)
|
||||||
}
|
}
|
||||||
if result.Action != "cancel" {
|
if result.Action != "cancel" {
|
||||||
t.Errorf("got action %q, want %q", result.Action, "cancel")
|
t.Errorf("got action %q, want %q", result.Action, "cancel")
|
||||||
|
|
|
||||||
|
|
@ -844,6 +844,33 @@ func (ss *ServerSession) updateState(mut func(*ServerSessionState)) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// hasInitialized reports whether the server has received the initialized
|
||||||
|
// notification.
|
||||||
|
//
|
||||||
|
// TODO(findleyr): use this to prevent change notifications.
|
||||||
|
func (ss *ServerSession) hasInitialized() bool {
|
||||||
|
ss.mu.Lock()
|
||||||
|
defer ss.mu.Unlock()
|
||||||
|
return ss.state.InitializedParams != nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// checkInitialized returns a formatted error if the server has not yet
|
||||||
|
// received the initialized notification.
|
||||||
|
func (ss *ServerSession) checkInitialized(method string) error {
|
||||||
|
if !ss.hasInitialized() {
|
||||||
|
// TODO(rfindley): enable this check.
|
||||||
|
// Right now is is flaky, because server tests don't await the initialized notification.
|
||||||
|
// Perhaps requests should simply block until they have received the initialized notification
|
||||||
|
|
||||||
|
// if strings.HasPrefix(method, "notifications/") {
|
||||||
|
// return fmt.Errorf("must not send %q before %q is received", method, notificationInitialized)
|
||||||
|
// } else {
|
||||||
|
// return fmt.Errorf("cannot call %q before %q is received", method, notificationInitialized)
|
||||||
|
// }
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func (ss *ServerSession) ID() string {
|
func (ss *ServerSession) ID() string {
|
||||||
if c, ok := ss.mcpConn.(hasSessionID); ok {
|
if c, ok := ss.mcpConn.(hasSessionID); ok {
|
||||||
return c.SessionID()
|
return c.SessionID()
|
||||||
|
|
@ -859,11 +886,17 @@ func (ss *ServerSession) Ping(ctx context.Context, params *PingParams) error {
|
||||||
|
|
||||||
// ListRoots lists the client roots.
|
// ListRoots lists the client roots.
|
||||||
func (ss *ServerSession) ListRoots(ctx context.Context, params *ListRootsParams) (*ListRootsResult, error) {
|
func (ss *ServerSession) ListRoots(ctx context.Context, params *ListRootsParams) (*ListRootsResult, error) {
|
||||||
|
if err := ss.checkInitialized(methodListRoots); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
return handleSend[*ListRootsResult](ctx, methodListRoots, newServerRequest(ss, orZero[Params](params)))
|
return handleSend[*ListRootsResult](ctx, methodListRoots, newServerRequest(ss, orZero[Params](params)))
|
||||||
}
|
}
|
||||||
|
|
||||||
// CreateMessage sends a sampling request to the client.
|
// CreateMessage sends a sampling request to the client.
|
||||||
func (ss *ServerSession) CreateMessage(ctx context.Context, params *CreateMessageParams) (*CreateMessageResult, error) {
|
func (ss *ServerSession) CreateMessage(ctx context.Context, params *CreateMessageParams) (*CreateMessageResult, error) {
|
||||||
|
if err := ss.checkInitialized(methodCreateMessage); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
if params == nil {
|
if params == nil {
|
||||||
params = &CreateMessageParams{Messages: []*SamplingMessage{}}
|
params = &CreateMessageParams{Messages: []*SamplingMessage{}}
|
||||||
}
|
}
|
||||||
|
|
@ -877,6 +910,9 @@ func (ss *ServerSession) CreateMessage(ctx context.Context, params *CreateMessag
|
||||||
|
|
||||||
// Elicit sends an elicitation request to the client asking for user input.
|
// Elicit sends an elicitation request to the client asking for user input.
|
||||||
func (ss *ServerSession) Elicit(ctx context.Context, params *ElicitParams) (*ElicitResult, error) {
|
func (ss *ServerSession) Elicit(ctx context.Context, params *ElicitParams) (*ElicitResult, error) {
|
||||||
|
if err := ss.checkInitialized(methodElicit); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
return handleSend[*ElicitResult](ctx, methodElicit, newServerRequest(ss, orZero[Params](params)))
|
return handleSend[*ElicitResult](ctx, methodElicit, newServerRequest(ss, orZero[Params](params)))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -978,7 +1014,7 @@ func (ss *ServerSession) getConn() *jsonrpc2.Connection { return ss.conn }
|
||||||
// handle invokes the method described by the given JSON RPC request.
|
// handle invokes the method described by the given JSON RPC request.
|
||||||
func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, error) {
|
func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, error) {
|
||||||
ss.mu.Lock()
|
ss.mu.Lock()
|
||||||
initialized := ss.state.InitializedParams != nil
|
initialized := ss.state.InitializeParams != nil
|
||||||
ss.mu.Unlock()
|
ss.mu.Unlock()
|
||||||
|
|
||||||
// From the spec:
|
// From the spec:
|
||||||
|
|
|
||||||
|
|
@ -346,9 +346,12 @@ func notifySessions[S Session, P Params](sessions []S, method string, params P)
|
||||||
if sessions == nil {
|
if sessions == nil {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
// TODO: make this timeout configurable, or call Notify asynchronously.
|
// TODO: make this timeout configurable, or call handleNotify asynchronously.
|
||||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
|
|
||||||
|
// TODO: there's a potential spec violation here, when the feature list
|
||||||
|
// changes before the session (client or server) is initialized.
|
||||||
for _, s := range sessions {
|
for _, s := range sessions {
|
||||||
req := newRequest(s, params)
|
req := newRequest(s, params)
|
||||||
if err := handleNotify(ctx, method, req); err != nil {
|
if err := handleNotify(ctx, method, req); err != nil {
|
||||||
|
|
|
||||||
14
mcp/testdata/conformance/server/lifecycle.txtar
vendored
14
mcp/testdata/conformance/server/lifecycle.txtar
vendored
|
|
@ -5,6 +5,7 @@ See also modelcontextprotocol/go-sdk#225.
|
||||||
|
|
||||||
-- client --
|
-- client --
|
||||||
{ "jsonrpc":"2.0", "method": "notifications/initialized" }
|
{ "jsonrpc":"2.0", "method": "notifications/initialized" }
|
||||||
|
{ "jsonrpc": "2.0", "id": 2, "method": "tools/list" }
|
||||||
{
|
{
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": 1,
|
"id": 1,
|
||||||
|
|
@ -21,6 +22,14 @@ See also modelcontextprotocol/go-sdk#225.
|
||||||
{ "jsonrpc": "2.0", "id": 3, "method": "tools/list" }
|
{ "jsonrpc": "2.0", "id": 3, "method": "tools/list" }
|
||||||
|
|
||||||
-- server --
|
-- server --
|
||||||
|
{
|
||||||
|
"jsonrpc": "2.0",
|
||||||
|
"id": 2,
|
||||||
|
"error": {
|
||||||
|
"code": 0,
|
||||||
|
"message": "method \"tools/list\" is invalid during session initialization"
|
||||||
|
}
|
||||||
|
}
|
||||||
{
|
{
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": 1,
|
"id": 1,
|
||||||
|
|
@ -43,9 +52,8 @@ See also modelcontextprotocol/go-sdk#225.
|
||||||
{
|
{
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": 2,
|
"id": 2,
|
||||||
"error": {
|
"result": {
|
||||||
"code": 0,
|
"tools": []
|
||||||
"message": "method \"tools/list\" is invalid during session initialization"
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue