Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions go/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,13 +620,26 @@ func (s *Session) executeToolAndRespond(requestID, toolName, toolCallID string,
return
}

resultStr := result.TextResultForLLM
if resultStr == "" {
resultStr = fmt.Sprintf("%v", result)
textResultForLLM := result.TextResultForLLM
if textResultForLLM == "" {
textResultForLLM = fmt.Sprintf("%v", result)
}

rpcResult := rpc.ResultUnion{
ResultResult: &rpc.ResultResult{
TextResultForLlm: textResultForLLM,
ToolTelemetry: result.ToolTelemetry,
},
}
if result.ResultType != "" {
rpcResult.ResultResult.ResultType = &result.ResultType
}
Comment thread
Morabbin marked this conversation as resolved.
Outdated
if result.Error != "" {
rpcResult.ResultResult.Error = &result.Error
}
s.RPC.Tools.HandlePendingToolCall(ctx, &rpc.SessionToolsHandlePendingToolCallParams{
RequestID: requestID,
Result: &rpc.ResultUnion{String: &resultStr},
Result: &rpcResult,
})
Comment thread
Morabbin marked this conversation as resolved.
}

Expand Down
21 changes: 20 additions & 1 deletion nodejs/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import type {
SessionUiApi,
Tool,
ToolHandler,
ToolResult,
ToolResultObject,
TraceContextProvider,
TypedSessionEventHandler,
UserInputHandler,
Expand Down Expand Up @@ -459,11 +461,13 @@ export class CopilotSession {
traceparent,
tracestate,
});
let result: string;
let result: ToolResult;
if (rawResult == null) {
result = "";
} else if (typeof rawResult === "string") {
result = rawResult;
} else if (isToolResultObject(rawResult)) {
result = rawResult;
} else {
result = JSON.stringify(rawResult);
Comment thread
Morabbin marked this conversation as resolved.
}
Expand Down Expand Up @@ -1043,3 +1047,18 @@ export class CopilotSession {
await this.rpc.log({ message, ...options });
}
}

/**
* Type guard that checks whether a value is a {@link ToolResultObject}.
* A valid object must have a string `textResultForLlm` and a string `resultType`.
*/
function isToolResultObject(value: unknown): value is ToolResultObject {
return (
typeof value === "object" &&
value !== null &&
"textResultForLlm" in value &&
typeof (value as ToolResultObject).textResultForLlm === "string" &&
"resultType" in value &&
typeof (value as ToolResultObject).resultType === "string"
);
Comment thread
Morabbin marked this conversation as resolved.
Outdated
}
51 changes: 49 additions & 2 deletions nodejs/test/e2e/tool_results.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

import { describe, expect, it } from "vitest";
import { z } from "zod";
import type { ToolResultObject } from "../../src/index.js";
import type { SessionEvent, ToolResultObject } from "../../src/index.js";
import { approveAll, defineTool } from "../../src/index.js";
import { createSdkTestContext } from "./harness/sdkTestContext";

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

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

await session.disconnect();
});

it("should preserve toolTelemetry and not stringify structured results for LLM", async () => {
const session = await client.createSession({
onPermissionRequest: approveAll,
tools: [
defineTool("analyze_code", {
description: "Analyzes code for issues",
parameters: z.object({
file: z.string(),
}),
handler: ({ file }): ToolResultObject => ({
textResultForLlm: `Analysis of ${file}: no issues found`,
resultType: "success",
toolTelemetry: {
metrics: { analysisTimeMs: 150 },
properties: { analyzer: "eslint" },
},
}),
}),
],
});

const events: SessionEvent[] = [];
session.on((event) => events.push(event));

const assistantMessage = await session.sendAndWait({
prompt: "Analyze the file main.ts for issues.",
});

expect(assistantMessage?.data.content).toMatch(/no issues/i);

// Verify the LLM received just textResultForLlm, not stringified JSON
const traffic = await openAiEndpoint.getExchanges();
const lastConversation = traffic[traffic.length - 1]!;
const toolResults = lastConversation.request.messages.filter(
(m: { role: string }) => m.role === "tool"
);
expect(toolResults.length).toBe(1);
expect(toolResults[0]!.content).not.toContain("toolTelemetry");
expect(toolResults[0]!.content).not.toContain("resultType");

// Verify tool.execution_complete event carries toolTelemetry
const toolCompletes = events.filter((e) => e.type === "tool.execution_complete");
expect(toolCompletes.length).toBeGreaterThanOrEqual(1);

Comment thread
Morabbin marked this conversation as resolved.
Outdated
await session.disconnect();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ conversations:
arguments: "{}"
- role: tool
tool_call_id: toolcall_0
content: '{"error":"API timeout","resultType":"failure","textResultForLlm":"Service unavailable"}'
content: Service unavailable
- role: assistant
content: service is down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
models:
- claude-sonnet-4.5
conversations:
- messages:
- role: system
content: ${system}
- role: user
content: Analyze the file main.ts for issues.
- role: assistant
tool_calls:
- id: toolcall_0
type: function
function:
name: analyze_code
arguments: '{"file":"main.ts"}'
- role: tool
tool_call_id: toolcall_0
content: "Analysis of main.ts: no issues found"
- role: assistant
content: "The analysis of main.ts is complete -- no issues were found."
Loading