From 03d78dfa659f378ad6af3064f161ce728aca793a Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Thu, 11 Jun 2026 16:03:50 +0300 Subject: [PATCH] fix: address review findings on SDK godoc and nil guard - pkg/kit: remove internal package paths from exported godoc on ParseTemplate and the ToolKind* constants (SDK doc surface must not reference internal packages) - internal/tools: guard marshalToolResult against a nil CallToolResult (json.Marshal(nil) succeeds as 'null', then result.IsError panics if a client returns nil result with nil error) Skipped the TreeNode Children deep-copy suggestion: the slice already comes from TreeManager.GetChildren which returns a fresh copy per call into a throwaway intermediate, so no internal state is exposed. --- internal/tools/mcp.go | 3 +++ pkg/kit/events.go | 4 ++-- pkg/kit/template_bridge.go | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/tools/mcp.go b/internal/tools/mcp.go index 09ae3111..d4ab10f0 100644 --- a/internal/tools/mcp.go +++ b/internal/tools/mcp.go @@ -737,6 +737,9 @@ func (m *MCPToolManager) withOAuthRetry(ctx context.Context, serverName, toolNam // marshalToolResult converts an MCP CallToolResult into the JSON-encoded // MCPToolResult shape returned to the agent. func marshalToolResult(result *mcp.CallToolResult) (*MCPToolResult, error) { + if result == nil { + return nil, errors.New("mcp tool call returned nil result") + } marshaled, err := json.Marshal(result) if err != nil { return nil, fmt.Errorf("failed to marshal mcp tool result: %w", err) diff --git a/pkg/kit/events.go b/pkg/kit/events.go index 11f3c0cc..d662f1df 100644 --- a/pkg/kit/events.go +++ b/pkg/kit/events.go @@ -106,8 +106,8 @@ type Event interface { // for execute tools) and file trackers to identify which results contain // modifications. // -// The canonical classification lives in internal/extensions; these constants -// re-export it so SDK events and extension events always agree. +// These constants re-export the canonical classification used by extension +// events, so SDK events and extension events always agree. const ( ToolKindExecute = extensions.ToolKindExecute // Shell execution (bash) ToolKindEdit = extensions.ToolKindEdit // File modification (edit, write) diff --git a/pkg/kit/template_bridge.go b/pkg/kit/template_bridge.go index 048c1a15..ba0b0c58 100644 --- a/pkg/kit/template_bridge.go +++ b/pkg/kit/template_bridge.go @@ -16,7 +16,8 @@ import ( // --------------------------------------------------------------------------- // ParseTemplate extracts {{variables}} from template content. The template -// grammar is shared with skill prompt templates (see internal/skills). +// grammar is shared with skill prompt templates, so a template parses +// identically regardless of which API loads it. func ParseTemplate(name, content string) extensions.PromptTemplate { tpl := skills.NewPromptTemplate(name, content) vars := tpl.Variables