Skip to content

Commit a2b4487

Browse files
authored
Merge pull request #1093 from dgageot/tool-errors
Make it possible to recognize tool call error vs success
2 parents 5a41b0e + 28a683e commit a2b4487

19 files changed

Lines changed: 118 additions & 183 deletions

File tree

examples/golibrary/tool/main.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,7 @@ func addNumbers(_ context.Context, toolCall tools.ToolCall) (*tools.ToolCallResu
4040

4141
fmt.Println("Adding numbers", p.A, p.B)
4242

43-
return &tools.ToolCallResult{
44-
Output: fmt.Sprintf("%d", p.A+p.B),
45-
}, nil
43+
return tools.ResultSuccess(fmt.Sprintf("%d", p.A+p.B)), nil
4644
}
4745

4846
func run(ctx context.Context) error {

pkg/acp/filesystem.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,18 @@ func (t *FilesystemToolset) handleReadFile(ctx context.Context, toolCall tools.T
7676

7777
sessionID, ok := getSessionID(ctx)
7878
if !ok {
79-
return &tools.ToolCallResult{Output: "Error: session ID not found in context"}, nil
79+
return tools.ResultError("Error: session ID not found in context"), nil
8080
}
8181

8282
resp, err := t.agent.conn.ReadTextFile(ctx, acp.ReadTextFileRequest{
8383
SessionId: acp.SessionId(sessionID),
8484
Path: filepath.Join(t.workindgDir, args.Path),
8585
})
8686
if err != nil {
87-
return &tools.ToolCallResult{Output: fmt.Sprintf("Error reading file: %s", err)}, nil
87+
return tools.ResultError(fmt.Sprintf("Error reading file: %s", err)), nil
8888
}
8989

90-
return &tools.ToolCallResult{Output: resp.Content}, nil
90+
return tools.ResultSuccess(resp.Content), nil
9191
}
9292

9393
func (t *FilesystemToolset) handleWriteFile(ctx context.Context, toolCall tools.ToolCall) (*tools.ToolCallResult, error) {
@@ -98,7 +98,7 @@ func (t *FilesystemToolset) handleWriteFile(ctx context.Context, toolCall tools.
9898

9999
sessionID, ok := getSessionID(ctx)
100100
if !ok {
101-
return &tools.ToolCallResult{Output: "Error: session ID not found in context"}, nil
101+
return tools.ResultError("Error: session ID not found in context"), nil
102102
}
103103

104104
_, err := t.agent.conn.WriteTextFile(ctx, acp.WriteTextFileRequest{
@@ -107,10 +107,10 @@ func (t *FilesystemToolset) handleWriteFile(ctx context.Context, toolCall tools.
107107
Content: args.Content,
108108
})
109109
if err != nil {
110-
return &tools.ToolCallResult{Output: fmt.Sprintf("Error writing file: %s", err)}, nil
110+
return tools.ResultError(fmt.Sprintf("Error writing file: %s", err)), nil
111111
}
112112

113-
return &tools.ToolCallResult{Output: "File written successfully"}, nil
113+
return tools.ResultSuccess("File written successfully"), nil
114114
}
115115

116116
func (t *FilesystemToolset) handleEditFile(ctx context.Context, toolCall tools.ToolCall) (*tools.ToolCallResult, error) {
@@ -121,22 +121,22 @@ func (t *FilesystemToolset) handleEditFile(ctx context.Context, toolCall tools.T
121121

122122
sessionID, ok := getSessionID(ctx)
123123
if !ok {
124-
return &tools.ToolCallResult{Output: "Error: session ID not found in context"}, nil
124+
return tools.ResultError("Error: session ID not found in context"), nil
125125
}
126126

127127
resp, err := t.agent.conn.ReadTextFile(ctx, acp.ReadTextFileRequest{
128128
SessionId: acp.SessionId(sessionID),
129129
Path: filepath.Join(t.workindgDir, args.Path),
130130
})
131131
if err != nil {
132-
return &tools.ToolCallResult{Output: fmt.Sprintf("Error reading file: %s", err)}, nil
132+
return tools.ResultError(fmt.Sprintf("Error reading file: %s", err)), nil
133133
}
134134

135135
modifiedContent := resp.Content
136136

137137
for i, edit := range args.Edits {
138138
if !strings.Contains(modifiedContent, edit.OldText) {
139-
return &tools.ToolCallResult{Output: fmt.Sprintf("Edit %d failed: old text not found", i+1)}, nil
139+
return tools.ResultError(fmt.Sprintf("Edit %d failed: old text not found", i+1)), nil
140140
}
141141
modifiedContent = strings.Replace(modifiedContent, edit.OldText, edit.NewText, 1)
142142
}
@@ -147,8 +147,8 @@ func (t *FilesystemToolset) handleEditFile(ctx context.Context, toolCall tools.T
147147
Content: modifiedContent,
148148
})
149149
if err != nil {
150-
return &tools.ToolCallResult{Output: fmt.Sprintf("Error writing file: %s", err)}, nil
150+
return tools.ResultError(fmt.Sprintf("Error writing file: %s", err)), nil
151151
}
152152

153-
return &tools.ToolCallResult{Output: "File edited successfully"}, nil
153+
return tools.ResultSuccess("File edited successfully"), nil
154154
}

pkg/runtime/runtime.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,15 +1125,13 @@ func (r *LocalRuntime) runTool(ctx context.Context, tool tools.Tool, toolCall to
11251125
if errors.Is(err, context.Canceled) || errors.Is(ctx.Err(), context.Canceled) {
11261126
slog.Debug("Tool handler canceled by context", "tool", toolCall.Function.Name, "agent", a.Name(), "session_id", sess.ID)
11271127
// Synthesize a cancellation response so the transcript remains consistent
1128-
res = &tools.ToolCallResult{Output: "The tool call was canceled by the user."}
1128+
res = tools.ResultError("The tool call was canceled by the user.")
11291129
span.SetStatus(codes.Ok, "tool handler canceled by user")
11301130
} else {
11311131
span.RecordError(err)
11321132
span.SetStatus(codes.Error, "tool handler error")
11331133
slog.Error("Error calling tool", "tool", toolCall.Function.Name, "error", err)
1134-
res = &tools.ToolCallResult{
1135-
Output: fmt.Sprintf("Error calling tool: %v", err),
1136-
}
1134+
res = tools.ResultError(fmt.Sprintf("Error calling tool: %v", err))
11371135
}
11381136
} else {
11391137
span.SetStatus(codes.Ok, "tool handler completed")
@@ -1346,9 +1344,7 @@ func (r *LocalRuntime) handleTaskTransfer(ctx context.Context, sess *session.Ses
13461344
slog.Debug("Task transfer completed", "agent", params.Agent, "task", params.Task)
13471345

13481346
span.SetStatus(codes.Ok, "task transfer completed")
1349-
return &tools.ToolCallResult{
1350-
Output: s.GetLastAssistantMessageContent(),
1351-
}, nil
1347+
return tools.ResultSuccess(s.GetLastAssistantMessageContent()), nil
13521348
}
13531349

13541350
func (r *LocalRuntime) handleHandoff(_ context.Context, _ *session.Session, toolCall tools.ToolCall, _ chan Event) (*tools.ToolCallResult, error) {
@@ -1364,9 +1360,7 @@ func (r *LocalRuntime) handleHandoff(_ context.Context, _ *session.Session, tool
13641360
}
13651361

13661362
r.currentAgent = next.Name()
1367-
return &tools.ToolCallResult{
1368-
Output: fmt.Sprintf("The agent %s handed off the conversation to you, look at the history of the conversation and continue where it left off. Once you are done with your task or if the user asks you, handoff the conversation back to %s.", ca, ca),
1369-
}, nil
1363+
return tools.ResultSuccess(fmt.Sprintf("The agent %s handed off the conversation to you, look at the history of the conversation and continue where it left off. Once you are done with your task or if the user asks you, handoff the conversation back to %s.", ca, ca)), nil
13701364
}
13711365

13721366
// Summarize generates a summary for the session based on the conversation history

pkg/teamloader/toon_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ import (
1212

1313
func mockHandler(output string) tools.ToolHandler {
1414
return func(ctx context.Context, toolCall tools.ToolCall) (*tools.ToolCallResult, error) {
15-
return &tools.ToolCallResult{
16-
Output: output,
17-
}, nil
15+
return tools.ResultSuccess(output), nil
1816
}
1917
}
2018

pkg/tools/a2a/a2a.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,8 @@ func (t *Toolset) createHandler() tools.ToolHandler {
179179
result = "No response from agent"
180180
}
181181

182-
return &tools.ToolCallResult{Output: result}, nil
182+
// TODO(dga): could this be a tool call error?
183+
return tools.ResultSuccess(result), nil
183184
}
184185
}
185186

pkg/tools/builtin/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func (h *apiHandler) CallTool(ctx context.Context, toolCall tools.ToolCall) (*to
8888
return nil, fmt.Errorf("failed to read response body: %v", err)
8989
}
9090

91-
return &tools.ToolCallResult{Output: limitOutput(string(body))}, nil
91+
return tools.ResultSuccess(limitOutput(string(body))), nil
9292
}
9393

9494
type APIToolOption func(*APITool)

pkg/tools/builtin/deferred.go

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,44 +107,34 @@ func (d *DeferredToolset) handleSearchTool(_ context.Context, args SearchToolArg
107107
}
108108

109109
if len(results) == 0 {
110-
return &tools.ToolCallResult{
111-
Output: fmt.Sprintf("No deferred tools found matching '%s'", args.Query),
112-
}, nil
110+
return tools.ResultError(fmt.Sprintf("No deferred tools found matching '%s'", args.Query)), nil
113111
}
114112

115113
output, err := json.MarshalIndent(results, "", " ")
116114
if err != nil {
117115
return nil, fmt.Errorf("failed to marshal results: %w", err)
118116
}
119117

120-
return &tools.ToolCallResult{
121-
Output: fmt.Sprintf("Found %d deferred tool(s):\n%s", len(results), string(output)),
122-
}, nil
118+
return tools.ResultSuccess(fmt.Sprintf("Found %d deferred tool(s):\n%s", len(results), string(output))), nil
123119
}
124120

125121
func (d *DeferredToolset) handleAddTool(_ context.Context, args AddToolArgs) (*tools.ToolCallResult, error) {
126122
d.mu.Lock()
127123
defer d.mu.Unlock()
128124

129125
if _, exists := d.activatedTools[args.Name]; exists {
130-
return &tools.ToolCallResult{
131-
Output: fmt.Sprintf("Tool '%s' is already active", args.Name),
132-
}, nil
126+
return tools.ResultSuccess(fmt.Sprintf("Tool '%s' is already active", args.Name)), nil
133127
}
134128

135129
entry, exists := d.deferredTools[args.Name]
136130
if !exists {
137-
return &tools.ToolCallResult{
138-
Output: fmt.Sprintf("Tool '%s' not found.", args.Name),
139-
}, nil
131+
return tools.ResultError(fmt.Sprintf("Tool '%s' not found.", args.Name)), nil
140132
}
141133

142134
delete(d.deferredTools, args.Name)
143135
d.activatedTools[args.Name] = entry.tool
144136

145-
return &tools.ToolCallResult{
146-
Output: fmt.Sprintf("Tool '%s' has been activated and is now available for use.\n\nDescription: %s", args.Name, entry.tool.Description),
147-
}, nil
137+
return tools.ResultSuccess(fmt.Sprintf("Tool '%s' has been activated and is now available for use.\n\nDescription: %s", args.Name, entry.tool.Description)), nil
148138
}
149139

150140
func (d *DeferredToolset) Tools(context.Context) ([]tools.Tool, error) {

pkg/tools/builtin/fetch.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,10 @@ func (h *fetchHandler) CallTool(ctx context.Context, params FetchToolArgs) (*too
6666
if len(params.URLs) == 1 {
6767
result := results[0]
6868
if result.Error != "" {
69-
return &tools.ToolCallResult{Output: fmt.Sprintf("Error fetching %s: %s", result.URL, result.Error)}, nil
69+
return tools.ResultError(fmt.Sprintf("Error fetching %s: %s", result.URL, result.Error)), nil
7070
}
71-
return &tools.ToolCallResult{
72-
Output: fmt.Sprintf("Successfully fetched %s (Status: %d, Length: %d bytes):\n\n%s",
73-
result.URL, result.StatusCode, result.ContentLength, result.Body),
74-
}, nil
71+
return tools.ResultSuccess(fmt.Sprintf("Successfully fetched %s (Status: %d, Length: %d bytes):\n\n%s",
72+
result.URL, result.StatusCode, result.ContentLength, result.Body)), nil
7573
}
7674

7775
// Multiple URLs - return structured results
@@ -80,7 +78,7 @@ func (h *fetchHandler) CallTool(ctx context.Context, params FetchToolArgs) (*too
8078
return nil, fmt.Errorf("failed to marshal results: %w", err)
8179
}
8280

83-
return &tools.ToolCallResult{Output: string(output)}, nil
81+
return tools.ResultSuccess(string(output)), nil
8482
}
8583

8684
type FetchResult struct {

0 commit comments

Comments
 (0)