Skip to content

Commit 4ad7b5a

Browse files
committed
refactor(core): agent session protocol changes
1 parent a809bc7 commit 4ad7b5a

7 files changed

Lines changed: 83 additions & 114 deletions

File tree

packages/core/src/agent/content-utils.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { describe, expect, it } from 'vitest';
88
import {
99
geminiPartsToContentParts,
1010
contentPartsToGeminiParts,
11+
toolResultDisplayToContentParts,
1112
buildToolResponseData,
1213
} from './content-utils.js';
1314
import type { Part } from '@google/genai';
@@ -199,6 +200,27 @@ describe('contentPartsToGeminiParts', () => {
199200
});
200201
});
201202

203+
describe('toolResultDisplayToContentParts', () => {
204+
it('returns undefined for undefined', () => {
205+
expect(toolResultDisplayToContentParts(undefined)).toBeUndefined();
206+
});
207+
208+
it('returns undefined for null', () => {
209+
expect(toolResultDisplayToContentParts(null)).toBeUndefined();
210+
});
211+
212+
it('handles string resultDisplay as-is', () => {
213+
const result = toolResultDisplayToContentParts('File written');
214+
expect(result).toEqual([{ type: 'text', text: 'File written' }]);
215+
});
216+
217+
it('stringifies object resultDisplay', () => {
218+
const display = { type: 'FileDiff', oldPath: 'a.ts', newPath: 'b.ts' };
219+
const result = toolResultDisplayToContentParts(display);
220+
expect(result).toEqual([{ type: 'text', text: JSON.stringify(display) }]);
221+
});
222+
});
223+
202224
describe('buildToolResponseData', () => {
203225
it('preserves outputFile and contentLength', () => {
204226
const result = buildToolResponseData({

packages/core/src/agent/content-utils.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import type { Part } from '@google/genai';
88
import type { ContentPart } from './types.js';
9+
import { debugLogger } from '../utils/debugLogger.js';
910

1011
/**
1112
* Converts Gemini API Part objects to framework-agnostic ContentPart objects.
@@ -93,6 +94,9 @@ export function contentPartsToGeminiParts(content: ContentPart[]): Part[] {
9394
result.push({ text: part.text });
9495
break;
9596
default:
97+
debugLogger.warn(
98+
`Unhandled ContentPart type: ${JSON.stringify(part)} fallback to serialization`,
99+
);
96100
// Serialize unknown ContentPart variants instead of dropping them
97101
result.push({ text: JSON.stringify(part) });
98102
break;
@@ -101,6 +105,24 @@ export function contentPartsToGeminiParts(content: ContentPart[]): Part[] {
101105
return result;
102106
}
103107

108+
/**
109+
* Converts a ToolCallResponseInfo.resultDisplay value into ContentPart[].
110+
* Handles string, object-valued (FileDiff, SubagentProgress, etc.),
111+
* and undefined resultDisplay consistently.
112+
*/
113+
export function toolResultDisplayToContentParts(
114+
resultDisplay: unknown,
115+
): ContentPart[] | undefined {
116+
if (resultDisplay === undefined || resultDisplay === null) {
117+
return undefined;
118+
}
119+
const text =
120+
typeof resultDisplay === 'string'
121+
? resultDisplay
122+
: JSON.stringify(resultDisplay);
123+
return [{ type: 'text', text }];
124+
}
125+
104126
/**
105127
* Builds the data record for a tool_response AgentEvent, preserving
106128
* all available metadata from the ToolCallResponseInfo.

packages/core/src/agent/event-translator.test.ts

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,9 @@ describe('translateEvent', () => {
155155
expect(resp.content).toEqual([
156156
{ type: 'text', text: 'Permission denied to write' },
157157
]);
158-
expect(resp.display?.result).toEqual({
159-
type: 'text',
160-
text: 'Permission denied',
161-
});
158+
expect(resp.displayContent).toEqual([
159+
{ type: 'text', text: 'Permission denied' },
160+
]);
162161
expect(resp.data).toEqual({ errorType: 'permission_denied' });
163162
});
164163

@@ -201,12 +200,9 @@ describe('translateEvent', () => {
201200
};
202201
const result = translateEvent(event, state);
203202
const resp = result[0] as AgentEvent<'tool_response'>;
204-
expect(resp.display?.result).toEqual({
205-
type: 'diff',
206-
path: '/tmp/test.txt',
207-
beforeText: 'a',
208-
afterText: 'b',
209-
});
203+
expect(resp.displayContent).toEqual([
204+
{ type: 'text', text: JSON.stringify(objectDisplay) },
205+
]);
210206
});
211207

212208
it('passes through string resultDisplay as-is', () => {
@@ -224,10 +220,9 @@ describe('translateEvent', () => {
224220
};
225221
const result = translateEvent(event, state);
226222
const resp = result[0] as AgentEvent<'tool_response'>;
227-
expect(resp.display?.result).toEqual({
228-
type: 'text',
229-
text: 'Command output text',
230-
});
223+
expect(resp.displayContent).toEqual([
224+
{ type: 'text', text: 'Command output text' },
225+
]);
231226
});
232227

233228
it('preserves outputFile and contentLength in data', () => {
@@ -378,7 +373,7 @@ describe('translateEvent', () => {
378373
expect(err.type).toBe('error');
379374
expect(err.fatal).toBe(false);
380375
expect(err._meta?.['code']).toBe('AGENT_EXECUTION_BLOCKED');
381-
expect(err.message).toBe('Policy violation');
376+
expect(err.message).toBe('Agent execution blocked: Policy violation');
382377
});
383378

384379
it('uses systemMessage in the final error message when available', () => {
@@ -393,7 +388,9 @@ describe('translateEvent', () => {
393388
};
394389
const result = translateEvent(event, state);
395390
const err = result[0] as AgentEvent<'error'>;
396-
expect(err.message).toBe('Blocked by policy hook');
391+
expect(err.message).toBe(
392+
'Agent execution blocked: Blocked by policy hook',
393+
);
397394
});
398395
});
399396

packages/core/src/agent/event-translator.ts

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,12 @@ import type {
2525
ErrorData,
2626
Usage,
2727
AgentEventType,
28-
ToolDisplay,
2928
} from './types.js';
3029
import {
3130
geminiPartsToContentParts,
31+
toolResultDisplayToContentParts,
3232
buildToolResponseData,
3333
} from './content-utils.js';
34-
import { toolResultDisplayToDisplayContent } from './tool-display-utils.js';
3534

3635
// ---------------------------------------------------------------------------
3736
// Translation State
@@ -210,7 +209,7 @@ export function translateEvent(
210209
out.push(
211210
makeEvent('error', state, {
212211
status: 'PERMISSION_DENIED',
213-
message: event.value.systemMessage?.trim() || event.value.reason,
212+
message: `Agent execution blocked: ${event.value.systemMessage?.trim() || event.value.reason}`,
214213
fatal: false,
215214
_meta: { code: 'AGENT_EXECUTION_BLOCKED' },
216215
}),
@@ -236,23 +235,16 @@ export function translateEvent(
236235
requestId: event.value.callId,
237236
name: event.value.name,
238237
args: event.value.args,
239-
display: event.value.display,
240238
}),
241239
);
242240
break;
243241

244242
case GeminiEventType.ToolCallResponse: {
245243
ensureStreamStart(state, out);
244+
const displayContent = toolResultDisplayToContentParts(
245+
event.value.resultDisplay,
246+
);
246247
const data = buildToolResponseData(event.value);
247-
const display: ToolDisplay | undefined =
248-
event.value.display ??
249-
(event.value.resultDisplay
250-
? {
251-
result: toolResultDisplayToDisplayContent(
252-
event.value.resultDisplay,
253-
),
254-
}
255-
: undefined);
256248
out.push(
257249
makeEvent('tool_response', state, {
258250
requestId: event.value.callId,
@@ -261,7 +253,7 @@ export function translateEvent(
261253
? [{ type: 'text', text: event.value.error.message }]
262254
: geminiPartsToContentParts(event.value.responseParts),
263255
isError: event.value.error !== undefined,
264-
...(display ? { display } : {}),
256+
...(displayContent ? { displayContent } : {}),
265257
...(data ? { data } : {}),
266258
}),
267259
);
@@ -282,6 +274,7 @@ export function translateEvent(
282274
((x: never) => {
283275
throw new Error(`Unhandled event type: ${JSON.stringify(x)}`);
284276
})(event);
277+
break;
285278
}
286279

287280
return out;

packages/core/src/agent/legacy-agent-session.test.ts

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,7 @@ function makeCompletedToolCall(
102102
response: {
103103
callId,
104104
responseParts: [{ text: responseText }],
105-
resultDisplay: responseText,
106-
display: {
107-
result: { type: 'text', text: responseText },
108-
},
105+
resultDisplay: undefined,
109106
error: undefined,
110107
errorType: undefined,
111108
},
@@ -203,6 +200,7 @@ describe('LegacyAgentSession', () => {
203200
expect.any(AbortSignal),
204201
'test-prompt',
205202
undefined,
203+
false,
206204
'raw input',
207205
);
208206

@@ -429,12 +427,6 @@ describe('LegacyAgentSession', () => {
429427
(e): e is AgentEvent<'tool_response'> => e.type === 'tool_response',
430428
);
431429
expect(toolResp?.name).toBe('read_file');
432-
expect(toolResp?.display).toEqual(
433-
expect.objectContaining({
434-
name: 'read_file',
435-
result: { type: 'text', text: 'file contents' },
436-
}),
437-
);
438430
expect(toolResp?.content).toEqual([
439431
{ type: 'text', text: 'file contents' },
440432
]);
@@ -497,10 +489,9 @@ describe('LegacyAgentSession', () => {
497489
expect(toolResp?.content).toEqual([
498490
{ type: 'text', text: 'Permission denied' },
499491
]);
500-
expect(toolResp?.display?.result).toEqual({
501-
type: 'text',
502-
text: 'Error display',
503-
});
492+
expect(toolResp?.displayContent).toEqual([
493+
{ type: 'text', text: 'Error display' },
494+
]);
504495
});
505496

506497
it('stops on STOP_EXECUTION tool error', async () => {
@@ -655,7 +646,7 @@ describe('LegacyAgentSession', () => {
655646
e.type === 'error' && e._meta?.['code'] === 'AGENT_EXECUTION_BLOCKED',
656647
);
657648
expect(blocked?.fatal).toBe(false);
658-
expect(blocked?.message).toBe('Blocked by hook');
649+
expect(blocked?.message).toBe('Agent execution blocked: Blocked by hook');
659650

660651
const messages = events.filter(
661652
(e): e is AgentEvent<'message'> =>

packages/core/src/agent/legacy-agent-session.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import {
2323
buildToolResponseData,
2424
contentPartsToGeminiParts,
2525
geminiPartsToContentParts,
26+
toolResultDisplayToContentParts,
2627
} from './content-utils.js';
27-
import { populateToolDisplay } from './tool-display-utils.js';
2828
import { AgentSession } from './agent-session.js';
2929
import {
3030
createTranslationState,
@@ -166,6 +166,7 @@ export class LegacyAgentProtocol implements AgentProtocol {
166166
} else {
167167
this._emitErrorAndAgentEnd(err);
168168
}
169+
} finally {
169170
this._clearActiveStream();
170171
}
171172
}
@@ -196,6 +197,7 @@ export class LegacyAgentProtocol implements AgentProtocol {
196197
this._abortController.signal,
197198
this._promptId,
198199
undefined,
200+
false,
199201
currentDisplayContent,
200202
);
201203
currentDisplayContent = undefined;
@@ -261,13 +263,9 @@ export class LegacyAgentProtocol implements AgentProtocol {
261263
const content: ContentPart[] = response.error
262264
? [{ type: 'text', text: response.error.message }]
263265
: geminiPartsToContentParts(response.responseParts);
264-
const display = populateToolDisplay({
265-
name: request.name,
266-
invocation: 'invocation' in tc ? tc.invocation : undefined,
267-
resultDisplay: response.resultDisplay,
268-
displayName: 'tool' in tc ? tc.tool?.displayName : undefined,
269-
display: response.display,
270-
});
266+
const displayContent = toolResultDisplayToContentParts(
267+
response.resultDisplay,
268+
);
271269
const data = buildToolResponseData(response);
272270

273271
this._emit([
@@ -276,7 +274,7 @@ export class LegacyAgentProtocol implements AgentProtocol {
276274
name: request.name,
277275
content,
278276
isError: response.error !== undefined,
279-
...(display ? { display } : {}),
277+
...(displayContent ? { displayContent } : {}),
280278
...(data ? { data } : {}),
281279
}),
282280
]);
@@ -390,6 +388,7 @@ export class LegacyAgentProtocol implements AgentProtocol {
390388
const meta: Record<string, unknown> = {};
391389
if (err instanceof Error) {
392390
meta['errorName'] = err.constructor.name;
391+
meta['stack'] = err.stack;
393392
if ('exitCode' in err && typeof err.exitCode === 'number') {
394393
meta['exitCode'] = err.exitCode;
395394
}

0 commit comments

Comments
 (0)