From c6efe64a65a6392305ddc2e57c1c8b11089d5819 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 22 Jul 2025 18:03:26 +0200 Subject: [PATCH] fix: handle nil writer in bash MCP tool (#18978) - Refactors the bash tool to use `io.Discard` instead of nil to avoid panics. - Enhances panic recovery in `codersdk/toolsdk/toolsdk.go` by adding stack trace information in development builds. When a panic occurs in a tool handler: - In development builds: The error includes the full stack trace for easier debugging - In production builds: A simpler error message is shown without the stack trace --- codersdk/toolsdk/bash.go | 5 ++--- codersdk/toolsdk/toolsdk.go | 11 ++++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/codersdk/toolsdk/bash.go b/codersdk/toolsdk/bash.go index 0df5f69aa7..e45ca6a49e 100644 --- a/codersdk/toolsdk/bash.go +++ b/codersdk/toolsdk/bash.go @@ -79,13 +79,12 @@ Examples: } // Wait for agent to be ready - err = cliui.Agent(ctx, nil, workspaceAgent.ID, cliui.AgentOptions{ + if err := cliui.Agent(ctx, io.Discard, workspaceAgent.ID, cliui.AgentOptions{ FetchInterval: 0, Fetch: deps.coderClient.WorkspaceAgent, FetchLogs: deps.coderClient.WorkspaceAgentLogsAfter, Wait: true, // Always wait for startup scripts - }) - if err != nil { + }); err != nil { return WorkspaceBashResult{}, xerrors.Errorf("agent not ready: %w", err) } diff --git a/codersdk/toolsdk/toolsdk.go b/codersdk/toolsdk/toolsdk.go index 6ef310f510..670b5af145 100644 --- a/codersdk/toolsdk/toolsdk.go +++ b/codersdk/toolsdk/toolsdk.go @@ -6,12 +6,14 @@ import ( "context" "encoding/json" "io" + "runtime/debug" "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/aisdk-go" + "github.com/coder/coder/v2/buildinfo" "github.com/coder/coder/v2/codersdk" ) @@ -122,7 +124,14 @@ func WithRecover(h GenericHandlerFunc) GenericHandlerFunc { return func(ctx context.Context, deps Deps, args json.RawMessage) (ret json.RawMessage, err error) { defer func() { if r := recover(); r != nil { - err = xerrors.Errorf("tool handler panic: %v", r) + if buildinfo.IsDev() { + // Capture stack trace in dev builds + stack := debug.Stack() + err = xerrors.Errorf("tool handler panic: %v\nstack trace:\n%s", r, stack) + } else { + // Simple error message in production builds + err = xerrors.Errorf("tool handler panic: %v", r) + } } }() return h(ctx, deps, args)