Skip to content

Commit d6f3861

Browse files
MorabbinCopilot
andcommitted
Address review feedback: restore Go fallback, add test, fix snapshot
- Go: restore fallback to fmt.Sprintf when TextResultForLLM is empty, preserving prior behavior for handlers that don't set it. - Node: add e2e test verifying toolTelemetry is not leaked into LLM content and that the tool.execution_complete event fires. - Update failure-resulttype snapshot to expect just textResultForLlm content instead of the old stringified JSON. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f412754 commit d6f3861

4 files changed

Lines changed: 76 additions & 4 deletions

File tree

go/session.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,9 +620,14 @@ func (s *Session) executeToolAndRespond(requestID, toolName, toolCallID string,
620620
return
621621
}
622622

623+
textResultForLLM := result.TextResultForLLM
624+
if textResultForLLM == "" {
625+
textResultForLLM = fmt.Sprintf("%v", result)
626+
}
627+
623628
rpcResult := rpc.ResultUnion{
624629
ResultResult: &rpc.ResultResult{
625-
TextResultForLlm: result.TextResultForLLM,
630+
TextResultForLlm: textResultForLLM,
626631
ToolTelemetry: result.ToolTelemetry,
627632
},
628633
}

nodejs/test/e2e/tool_results.test.ts

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44

55
import { describe, expect, it } from "vitest";
66
import { z } from "zod";
7-
import type { ToolResultObject } from "../../src/index.js";
7+
import type { SessionEvent, ToolResultObject } from "../../src/index.js";
88
import { approveAll, defineTool } from "../../src/index.js";
99
import { createSdkTestContext } from "./harness/sdkTestContext";
1010

1111
describe("Tool Results", async () => {
12-
const { copilotClient: client } = await createSdkTestContext();
12+
const { copilotClient: client, openAiEndpoint } = await createSdkTestContext();
1313

1414
it("should handle structured ToolResultObject from custom tool", async () => {
1515
const session = await client.createSession({
@@ -98,4 +98,51 @@ describe("Tool Results", async () => {
9898

9999
await session.disconnect();
100100
});
101+
102+
it("should preserve toolTelemetry and not stringify structured results for LLM", async () => {
103+
const session = await client.createSession({
104+
onPermissionRequest: approveAll,
105+
tools: [
106+
defineTool("analyze_code", {
107+
description: "Analyzes code for issues",
108+
parameters: z.object({
109+
file: z.string(),
110+
}),
111+
handler: ({ file }): ToolResultObject => ({
112+
textResultForLlm: `Analysis of ${file}: no issues found`,
113+
resultType: "success",
114+
toolTelemetry: {
115+
metrics: { analysisTimeMs: 150 },
116+
properties: { analyzer: "eslint" },
117+
},
118+
}),
119+
}),
120+
],
121+
});
122+
123+
const events: SessionEvent[] = [];
124+
session.on((event) => events.push(event));
125+
126+
const assistantMessage = await session.sendAndWait({
127+
prompt: "Analyze the file main.ts for issues.",
128+
});
129+
130+
expect(assistantMessage?.data.content).toMatch(/no issues/i);
131+
132+
// Verify the LLM received just textResultForLlm, not stringified JSON
133+
const traffic = await openAiEndpoint.getExchanges();
134+
const lastConversation = traffic[traffic.length - 1]!;
135+
const toolResults = lastConversation.request.messages.filter(
136+
(m: { role: string }) => m.role === "tool"
137+
);
138+
expect(toolResults.length).toBe(1);
139+
expect(toolResults[0]!.content).not.toContain("toolTelemetry");
140+
expect(toolResults[0]!.content).not.toContain("resultType");
141+
142+
// Verify tool.execution_complete event carries toolTelemetry
143+
const toolCompletes = events.filter((e) => e.type === "tool.execution_complete");
144+
expect(toolCompletes.length).toBeGreaterThanOrEqual(1);
145+
146+
await session.disconnect();
147+
});
101148
});

test/snapshots/tool_results/should_handle_tool_result_with_failure_resulttype.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@ conversations:
1515
arguments: "{}"
1616
- role: tool
1717
tool_call_id: toolcall_0
18-
content: '{"error":"API timeout","resultType":"failure","textResultForLlm":"Service unavailable"}'
18+
content: Service unavailable
1919
- role: assistant
2020
content: service is down
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
models:
2+
- claude-sonnet-4.5
3+
conversations:
4+
- messages:
5+
- role: system
6+
content: ${system}
7+
- role: user
8+
content: Analyze the file main.ts for issues.
9+
- role: assistant
10+
tool_calls:
11+
- id: toolcall_0
12+
type: function
13+
function:
14+
name: analyze_code
15+
arguments: '{"file":"main.ts"}'
16+
- role: tool
17+
tool_call_id: toolcall_0
18+
content: "Analysis of main.ts: no issues found"
19+
- role: assistant
20+
content: "The analysis of main.ts is complete -- no issues were found."

0 commit comments

Comments
 (0)