From bcd5c7c948394f0baed6508dc3fd1fb92bd80580 Mon Sep 17 00:00:00 2001 From: manjunathbhaskar Date: Thu, 23 Jul 2026 15:09:38 +0200 Subject: [PATCH] fix(server): recover panics in executeRegularToolAsTask (hybrid task mode) --- server/regular_tool_as_task_panic_test.go | 56 +++++++++++++++++++++++ server/server.go | 6 +++ 2 files changed, 62 insertions(+) create mode 100644 server/regular_tool_as_task_panic_test.go diff --git a/server/regular_tool_as_task_panic_test.go b/server/regular_tool_as_task_panic_test.go new file mode 100644 index 000000000..379163896 --- /dev/null +++ b/server/regular_tool_as_task_panic_test.go @@ -0,0 +1,56 @@ +package server + +import ( + "context" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestExecuteRegularToolAsTask_PanicRecovery mirrors +// TestExecuteTaskTool_PanicRecovery (added in #880) for the sibling code +// path: a regular ServerTool with TaskSupportOptional invoked asynchronously +// via the hybrid task mode. That fix covered executeTaskTool but did not +// touch executeRegularToolAsTask, which still ran without panic recovery -- +// an unrecovered panic in this goroutine crashes the whole server process. +func TestExecuteRegularToolAsTask_PanicRecovery(t *testing.T) { + s := NewMCPServer("test", "1.0.0") + + regularTool := ServerTool{ + Tool: mcp.Tool{ + Name: "panic-regular-tool", + Description: "A regular tool that panics when run as a task", + }, + Handler: func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + panic("deliberate panic in regular tool handler run as a task") + }, + } + + ctx := t.Context() + taskID := "test-regular-panic-task" + entry, err := s.createTask(ctx, taskID, "panic-regular-tool", nil, nil) + require.NoError(t, err) + + request := mcp.CallToolRequest{} + request.Params.Name = "panic-regular-tool" + + // Execute in a goroutine, same as the production hybrid-mode path. + go s.executeRegularToolAsTask(ctx, entry, regularTool, request) + + select { + case <-entry.done: + // Task completed without crashing the process. + case <-time.After(5 * time.Second): + t.Fatal("task did not complete within timeout; panic recovery may have failed") + } + + s.tasksMu.RLock() + assert.True(t, entry.completed) + assert.Equal(t, mcp.TaskStatusFailed, entry.task.Status) + assert.Contains(t, entry.task.StatusMessage, "panic in task tool handler") + assert.Contains(t, entry.task.StatusMessage, "deliberate panic in regular tool handler run as a task") + s.tasksMu.RUnlock() +} diff --git a/server/server.go b/server/server.go index c9a9d0797..4f2a89619 100644 --- a/server/server.go +++ b/server/server.go @@ -2222,6 +2222,12 @@ func (s *MCPServer) executeRegularToolAsTask( regularTool ServerTool, request mcp.CallToolRequest, ) { + defer func() { + if r := recover(); r != nil { + s.completeTask(entry, nil, fmt.Errorf("panic in task tool handler %s: %v", request.Params.Name, r)) + } + }() + // Create cancellable context for this task execution taskCtx, cancel := context.WithCancel(ctx) defer cancel()