From 3d6cbed882637478353170d402063b94d2bdc0f0 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 16 Jul 2026 12:23:59 +0530 Subject: [PATCH 1/3] feat(SANDBOX-1813): implement bash MCP tool handler - Add pkg/tools with BashTool handler, BashInput/BashOutput types, and CommandExecutor interface (satisfied by *session.SessionManager) - Extract X-Session-ID from request headers; reject if missing - Clamp timeout: default 60s, max 300s - Non-zero exit codes return IsError with structured output retained - Infrastructure failures return tool-level errors (option A) - Unit tests with mock executor covering all AC scenarios - Add github.com/modelcontextprotocol/go-sdk v1.4.0 dependency Co-authored-by: Cursor Signed-off-by: Feny Mehta --- go.mod | 5 + go.sum | 16 ++- pkg/tools/bash.go | 113 +++++++++++++++++++++ pkg/tools/bash_test.go | 220 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 352 insertions(+), 2 deletions(-) create mode 100644 pkg/tools/bash.go create mode 100644 pkg/tools/bash_test.go diff --git a/go.mod b/go.mod index 410d579..9a2aa92 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ toolchain go1.24.13 require ( github.com/google/uuid v1.6.0 + github.com/modelcontextprotocol/go-sdk v1.4.0 github.com/stretchr/testify v1.11.1 k8s.io/api v0.33.4 k8s.io/apimachinery v0.33.4 @@ -23,6 +24,7 @@ require ( github.com/gogo/protobuf v1.3.2 // indirect github.com/google/gnostic-models v0.6.9 // indirect github.com/google/go-cmp v0.7.0 // indirect + github.com/google/jsonschema-go v0.4.2 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/mailru/easyjson v0.7.7 // indirect @@ -30,7 +32,10 @@ require ( github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect + github.com/segmentio/asm v1.1.3 // indirect + github.com/segmentio/encoding v0.5.3 // indirect github.com/x448/float16 v0.8.4 // indirect + github.com/yosida95/uritemplate/v3 v3.0.2 // indirect golang.org/x/net v0.49.0 // indirect golang.org/x/oauth2 v0.34.0 // indirect golang.org/x/sys v0.40.0 // indirect diff --git a/go.sum b/go.sum index 2b61a20..431e2e2 100644 --- a/go.sum +++ b/go.sum @@ -21,12 +21,16 @@ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1v github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= +github.com/golang-jwt/jwt/v5 v5.3.0 h1:pv4AsKCKKZuqlgs5sUmn4x8UlGa0kEVt/puTpKx9vvo= +github.com/golang-jwt/jwt/v5 v5.3.0/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/google/gnostic-models v0.6.9 h1:MU/8wDLif2qCXZmzncUQ/BOfxWfthHi63KqpoNbWqVw= github.com/google/gnostic-models v0.6.9/go.mod h1:CiWsm0s6BSQd1hRn8/QmxqB6BesYcbSZxsz9b0KuDBw= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= +github.com/google/jsonschema-go v0.4.2 h1:tmrUohrwoLZZS/P3x7ex0WAVknEkBZM46iALbcqoRA8= +github.com/google/jsonschema-go v0.4.2/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db h1:097atOisP2aRj7vFgYQBbFN4U4JNXUNYpxael3UzMyo= github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db/go.mod h1:vavhavw2zAxS5dIdcRluK6cSGGPlZynqzFM8NdvU144= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -46,6 +50,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= +github.com/modelcontextprotocol/go-sdk v1.4.0 h1:u0kr8lbJc1oBcawK7Df+/ajNMpIDFE41OEPxdeTLOn8= +github.com/modelcontextprotocol/go-sdk v1.4.0/go.mod h1:Nxc2n+n/GdCebUaqCOhTetptS17SXXNu9IfNTaLDi1E= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -63,6 +69,10 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRI github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= +github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= +github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg= +github.com/segmentio/encoding v0.5.3 h1:OjMgICtcSFuNvQCdwqMCv9Tg7lEOXGwm1J5RPQccx6w= +github.com/segmentio/encoding v0.5.3/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -78,6 +88,8 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= +github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= +github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= @@ -113,8 +125,8 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.40.0 h1:yLkxfA+Qnul4cs9QA3KnlFu0lVmd8JJfoq+E41uSutA= -golang.org/x/tools v0.40.0/go.mod h1:Ik/tzLRlbscWpqqMRjyWYDisX8bG13FrdXp3o4Sr9lc= +golang.org/x/tools v0.41.0 h1:a9b8iMweWG+S0OBnlU36rzLp20z1Rp10w+IY2czHTQc= +golang.org/x/tools v0.41.0/go.mod h1:XSY6eDqxVNiYgezAVqqCeihT4j1U2CCsqvH3WhQpnlg= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/pkg/tools/bash.go b/pkg/tools/bash.go new file mode 100644 index 0000000..7c4c474 --- /dev/null +++ b/pkg/tools/bash.go @@ -0,0 +1,113 @@ +package tools + +import ( + "context" + "fmt" + + "github.com/codeready-toolchain/cli-mcp-server/pkg/agent" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// BashInput is the tool input schema for the bash MCP tool. +type BashInput struct { + Command string `json:"command" jsonschema:"Shell command to execute (full bash — pipes, redirects, chaining supported)"` + Timeout *int `json:"timeout,omitempty" jsonschema:"Max execution time in seconds (default 60, max 300)"` +} + +// BashOutput is the structured result returned to the LLM. +type BashOutput struct { + Stdout string `json:"stdout"` + Stderr string `json:"stderr"` + ExitCode int `json:"exit_code"` + DurationMs int64 `json:"duration_ms"` +} + +// CommandExecutor abstracts the session manager's ExecuteCommand for testability. +// *session.SessionManager satisfies this interface without an adapter. +type CommandExecutor interface { + ExecuteCommand(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) +} + +const ( + defaultTimeout = 60 + maxTimeout = 300 +) + +// BashTool implements the bash MCP tool handler. +type BashTool struct { + executor CommandExecutor + tool *mcp.Tool +} + +// NewBashTool creates a BashTool with the given executor. +func NewBashTool(executor CommandExecutor) *BashTool { + return &BashTool{ + executor: executor, + tool: &mcp.Tool{ + Name: "bash", + Description: "Execute a shell command in a persistent sandbox bash session. Supports full bash syntax: pipes, redirects, chaining, and standard Unix tools.", + }, + } +} + +// RegisterWith registers the bash tool on the given MCP server. +func (t *BashTool) RegisterWith(s *mcp.Server) { + mcp.AddTool(s, t.tool, t.handle) +} + +// Tool returns the underlying mcp.Tool definition. +func (t *BashTool) Tool() *mcp.Tool { + return t.tool +} + +func (t *BashTool) handle(ctx context.Context, req *mcp.CallToolRequest, input BashInput) (*mcp.CallToolResult, BashOutput, error) { + sessionID, err := extractSessionID(req) + if err != nil { + return nil, BashOutput{}, err + } + + if input.Command == "" { + return nil, BashOutput{}, fmt.Errorf("command is required") + } + + timeout := clampTimeout(input.Timeout) + + resp, err := t.executor.ExecuteCommand(ctx, sessionID, input.Command, timeout) + if err != nil { + return nil, BashOutput{}, fmt.Errorf("sandbox exec failed: %w", err) + } + + output := BashOutput{ + Stdout: resp.Stdout, + Stderr: resp.Stderr, + ExitCode: resp.ExitCode, + DurationMs: resp.DurationMs, + } + + if resp.ExitCode != 0 { + return &mcp.CallToolResult{IsError: true}, output, nil + } + return nil, output, nil +} + +func extractSessionID(req *mcp.CallToolRequest) (string, error) { + if req.Extra == nil || req.Extra.Header == nil { + return "", fmt.Errorf("missing X-Session-ID header") + } + sid := req.Extra.Header.Get("X-Session-ID") + if sid == "" { + return "", fmt.Errorf("missing X-Session-ID header") + } + return sid, nil +} + +func clampTimeout(t *int) int { + if t == nil || *t <= 0 { + return defaultTimeout + } + if *t > maxTimeout { + return maxTimeout + } + return *t +} + diff --git a/pkg/tools/bash_test.go b/pkg/tools/bash_test.go new file mode 100644 index 0000000..191cb80 --- /dev/null +++ b/pkg/tools/bash_test.go @@ -0,0 +1,220 @@ +package tools + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/codeready-toolchain/cli-mcp-server/pkg/agent" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type mockExecutor struct { + execFn func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) +} + +func (m *mockExecutor) ExecuteCommand(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + return m.execFn(ctx, sessionID, command, timeoutSec) +} + +func intPtr(i int) *int { return &i } + +func newRequestWithSessionID(sessionID string) *mcp.CallToolRequest { + h := http.Header{} + h.Set("X-Session-ID", sessionID) + return &mcp.CallToolRequest{ + Extra: &mcp.RequestExtra{Header: h}, + } +} + +func TestBashToolHandle(t *testing.T) { + t.Run("missing session header — nil Extra", func(t *testing.T) { + // given + executor := &mockExecutor{execFn: func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + t.Fatal("executor should not be called") + return nil, nil + }} + tool := NewBashTool(executor) + req := &mcp.CallToolRequest{Extra: nil} + + // when + _, _, err := tool.handle(context.Background(), req, BashInput{Command: "echo hi"}) + + // then + require.Error(t, err) + assert.Contains(t, err.Error(), "missing X-Session-ID header") + }) + + t.Run("missing session header — empty value", func(t *testing.T) { + // given + executor := &mockExecutor{execFn: func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + t.Fatal("executor should not be called") + return nil, nil + }} + tool := NewBashTool(executor) + req := &mcp.CallToolRequest{ + Extra: &mcp.RequestExtra{Header: http.Header{}}, + } + + // when + _, _, err := tool.handle(context.Background(), req, BashInput{Command: "echo hi"}) + + // then + require.Error(t, err) + assert.Contains(t, err.Error(), "missing X-Session-ID header") + }) + + t.Run("empty command", func(t *testing.T) { + // given + executor := &mockExecutor{execFn: func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + t.Fatal("executor should not be called") + return nil, nil + }} + tool := NewBashTool(executor) + req := newRequestWithSessionID("inv-123") + + // when + _, _, err := tool.handle(context.Background(), req, BashInput{Command: ""}) + + // then + require.Error(t, err) + assert.Contains(t, err.Error(), "command is required") + }) + + t.Run("successful command — exit 0", func(t *testing.T) { + // given + var gotSessionID, gotCommand string + var gotTimeout int + executor := &mockExecutor{execFn: func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + gotSessionID = sessionID + gotCommand = command + gotTimeout = timeoutSec + return &agent.ExecResponse{ + Stdout: "hello", + Stderr: "", + ExitCode: 0, + DurationMs: 42, + }, nil + }} + tool := NewBashTool(executor) + req := newRequestWithSessionID("inv-abc") + + // when + result, output, err := tool.handle(context.Background(), req, BashInput{Command: "echo hello", Timeout: intPtr(30)}) + + // then + require.NoError(t, err) + assert.Nil(t, result) + assert.Equal(t, "hello", output.Stdout) + assert.Equal(t, "", output.Stderr) + assert.Equal(t, 0, output.ExitCode) + assert.Equal(t, int64(42), output.DurationMs) + assert.Equal(t, "inv-abc", gotSessionID) + assert.Equal(t, "echo hello", gotCommand) + assert.Equal(t, 30, gotTimeout) + }) + + t.Run("non-zero exit code — IsError true with output", func(t *testing.T) { + // given + executor := &mockExecutor{execFn: func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + return &agent.ExecResponse{ + Stdout: "", + Stderr: "not found", + ExitCode: 1, + DurationMs: 5, + }, nil + }} + tool := NewBashTool(executor) + req := newRequestWithSessionID("inv-def") + + // when + result, output, err := tool.handle(context.Background(), req, BashInput{Command: "oc get pod missing"}) + + // then + require.NoError(t, err) + require.NotNil(t, result) + assert.True(t, result.IsError) + assert.Equal(t, "not found", output.Stderr) + assert.Equal(t, 1, output.ExitCode) + assert.Equal(t, int64(5), output.DurationMs) + }) + + t.Run("executor error — infrastructure failure", func(t *testing.T) { + // given + executor := &mockExecutor{execFn: func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + return nil, fmt.Errorf("resolve pod: connection refused") + }} + tool := NewBashTool(executor) + req := newRequestWithSessionID("inv-ghi") + + // when + _, _, err := tool.handle(context.Background(), req, BashInput{Command: "ls"}) + + // then + require.Error(t, err) + assert.Contains(t, err.Error(), "sandbox exec failed") + assert.Contains(t, err.Error(), "connection refused") + }) + + t.Run("session ID forwarded unchanged", func(t *testing.T) { + // given + var gotSessionID string + executor := &mockExecutor{execFn: func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + gotSessionID = sessionID + return &agent.ExecResponse{ExitCode: 0}, nil + }} + tool := NewBashTool(executor) + req := newRequestWithSessionID("investigation-xyz-789") + + // when + _, _, err := tool.handle(context.Background(), req, BashInput{Command: "pwd"}) + + // then + require.NoError(t, err) + assert.Equal(t, "investigation-xyz-789", gotSessionID) + }) +} + +func TestClampTimeout(t *testing.T) { + tests := []struct { + name string + input *int + expected int + }{ + {"nil defaults to 60", nil, 60}, + {"zero defaults to 60", intPtr(0), 60}, + {"negative defaults to 60", intPtr(-5), 60}, + {"within range passes through", intPtr(120), 120}, + {"max boundary passes through", intPtr(300), 300}, + {"above max clamped to 300", intPtr(500), 300}, + {"one is valid", intPtr(1), 1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // when + result := clampTimeout(tt.input) + + // then + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestRegisterWith(t *testing.T) { + // given + executor := &mockExecutor{execFn: func(ctx context.Context, sessionID, command string, timeoutSec int) (*agent.ExecResponse, error) { + return &agent.ExecResponse{ExitCode: 0}, nil + }} + tool := NewBashTool(executor) + server := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.1"}, nil) + + // when + tool.RegisterWith(server) + + // then + assert.Equal(t, "bash", tool.Tool().Name) +} From 84a192dc1c937bb61357d28d3f3575cc6d37b722 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 16 Jul 2026 14:06:41 +0530 Subject: [PATCH 2/3] fix: verify tool registration via ListTools client API - Address CodeRabbit review: TestRegisterWith now connects a client via in-memory transport and asserts the server exposes "bash" tool - Test fails if RegisterWith is a no-op Co-authored-by: Cursor Signed-off-by: Feny Mehta --- pkg/tools/bash_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/tools/bash_test.go b/pkg/tools/bash_test.go index 191cb80..3d80a46 100644 --- a/pkg/tools/bash_test.go +++ b/pkg/tools/bash_test.go @@ -215,6 +215,18 @@ func TestRegisterWith(t *testing.T) { // when tool.RegisterWith(server) - // then - assert.Equal(t, "bash", tool.Tool().Name) + // then — verify the server actually exposes the tool via ListTools + ct, st := mcp.NewInMemoryTransports() + _, err := server.Connect(context.Background(), st, nil) + require.NoError(t, err) + + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "0.1"}, nil) + cs, err := client.Connect(context.Background(), ct, nil) + require.NoError(t, err) + t.Cleanup(func() { cs.Close() }) + + result, err := cs.ListTools(context.Background(), nil) + require.NoError(t, err) + require.Len(t, result.Tools, 1) + assert.Equal(t, "bash", result.Tools[0].Name) } From 847d8530ac4109317e6177ab3d5bfdb4c7b30aee Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 16 Jul 2026 14:50:06 +0530 Subject: [PATCH 3/3] fix: close server and client sessions in TestRegisterWith - Capture *ServerSession from server.Connect and close in t.Cleanup - Check and report Close() errors for both server and client sessions - Addresses CodeRabbit review: errcheck lint and resource leak Co-authored-by: Cursor Signed-off-by: Feny Mehta --- pkg/tools/bash_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/tools/bash_test.go b/pkg/tools/bash_test.go index 3d80a46..2c0def8 100644 --- a/pkg/tools/bash_test.go +++ b/pkg/tools/bash_test.go @@ -217,13 +217,22 @@ func TestRegisterWith(t *testing.T) { // then — verify the server actually exposes the tool via ListTools ct, st := mcp.NewInMemoryTransports() - _, err := server.Connect(context.Background(), st, nil) + ss, err := server.Connect(context.Background(), st, nil) require.NoError(t, err) + t.Cleanup(func() { + if err := ss.Close(); err != nil { + t.Errorf("ServerSession.Close: %v", err) + } + }) client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "0.1"}, nil) cs, err := client.Connect(context.Background(), ct, nil) require.NoError(t, err) - t.Cleanup(func() { cs.Close() }) + t.Cleanup(func() { + if err := cs.Close(); err != nil { + t.Errorf("ClientSession.Close: %v", err) + } + }) result, err := cs.ListTools(context.Background(), nil) require.NoError(t, err)