Skip to content

Commit 3d51422

Browse files
adamfweidmanadamfweidman
committed
fix(core): address jacob's review comments on local-invocation (license year, description truncation, callId parallel tracking)
1 parent 3112697 commit 3d51422

4 files changed

Lines changed: 84 additions & 101 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ describe('contentPartsToGeminiParts', () => {
192192
const content = [
193193
{ type: 'custom_widget', payload: 123 },
194194
] as unknown as ContentPart[];
195+
195196
const warnSpy = vi.spyOn(debugLogger, 'warn');
196197
const result = contentPartsToGeminiParts(content);
197198

‎packages/core/src/agents/local-invocation.ts‎

Lines changed: 35 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ import {
3535
} from '../utils/agent-sanitization-utils.js';
3636
import { debugLogger } from '../utils/debugLogger.js';
3737

38-
const INPUT_PREVIEW_MAX_LENGTH = 50;
39-
const DESCRIPTION_MAX_LENGTH = 200;
40-
4138
/**
4239
* Represents a validated, executable instance of a subagent tool.
4340
*
@@ -80,14 +77,10 @@ export class LocalSubagentInvocation extends BaseToolInvocation<
8077
*/
8178
getDescription(): string {
8279
const inputSummary = Object.entries(this.params)
83-
.map(
84-
([key, value]) =>
85-
`${key}: ${String(value).slice(0, INPUT_PREVIEW_MAX_LENGTH)}`,
86-
)
80+
.map(([key, value]) => `${key}: ${String(value)}`)
8781
.join(', ');
8882

89-
const description = `Running subagent '${this.definition.name}' with inputs: { ${inputSummary} }`;
90-
return description.slice(0, DESCRIPTION_MAX_LENGTH);
83+
return `Running subagent '${this.definition.name}' with inputs: { ${inputSummary} }`;
9184
}
9285

9386
private publishActivity(activity: SubagentActivityItem): void {
@@ -168,8 +161,11 @@ export class LocalSubagentInvocation extends BaseToolInvocation<
168161
const args = JSON.stringify(
169162
sanitizeToolArgs(activity.data['args']),
170163
);
164+
const callId = activity.data['callId']
165+
? String(activity.data['callId'])
166+
: randomUUID();
171167
recentActivity.push({
172-
id: randomUUID(),
168+
id: callId,
173169
type: 'tool_call',
174170
content: name,
175171
displayName,
@@ -186,23 +182,28 @@ export class LocalSubagentInvocation extends BaseToolInvocation<
186182
break;
187183
}
188184
case 'TOOL_CALL_END': {
189-
const name = String(activity.data['name']);
190185
const data = activity.data['data'];
191186
const isError = isToolActivityError(data);
192187

193-
for (let i = recentActivity.length - 1; i >= 0; i--) {
194-
if (
195-
recentActivity[i].type === 'tool_call' &&
196-
recentActivity[i].content === name &&
197-
recentActivity[i].status === SubagentState.RUNNING
198-
) {
199-
recentActivity[i].status = isError
200-
? SubagentState.ERROR
201-
: SubagentState.COMPLETED;
202-
updated = true;
203-
204-
this.publishActivity(recentActivity[i]);
205-
break;
188+
const callId = activity.data['id']
189+
? String(activity.data['id'])
190+
: undefined;
191+
192+
if (callId) {
193+
for (let i = recentActivity.length - 1; i >= 0; i--) {
194+
if (
195+
recentActivity[i].type === 'tool_call' &&
196+
recentActivity[i].id === callId &&
197+
recentActivity[i].status === SubagentState.RUNNING
198+
) {
199+
recentActivity[i].status = isError
200+
? SubagentState.ERROR
201+
: SubagentState.COMPLETED;
202+
updated = true;
203+
204+
this.publishActivity(recentActivity[i]);
205+
break;
206+
}
206207
}
207208
}
208209
break;
@@ -218,31 +219,23 @@ export class LocalSubagentInvocation extends BaseToolInvocation<
218219
errorType === SubagentActivityErrorType.REJECTED ||
219220
error.startsWith(SUBAGENT_REJECTED_ERROR_PREFIX);
220221

221-
const toolName = activity.data['name']
222-
? String(activity.data['name'])
222+
const callId = activity.data['callId']
223+
? String(activity.data['callId'])
223224
: undefined;
224225

225-
if (toolName && (isCancellation || isRejection)) {
226-
for (let i = recentActivity.length - 1; i >= 0; i--) {
227-
if (
228-
recentActivity[i].type === 'tool_call' &&
229-
recentActivity[i].content === toolName &&
230-
recentActivity[i].status === SubagentState.RUNNING
231-
) {
232-
recentActivity[i].status = SubagentState.CANCELLED;
233-
updated = true;
234-
break;
235-
}
236-
}
237-
} else if (toolName) {
238-
// Mark non-rejection/non-cancellation errors as 'error'
226+
if (callId) {
227+
const targetStatus =
228+
isCancellation || isRejection
229+
? SubagentState.CANCELLED
230+
: SubagentState.ERROR;
231+
239232
for (let i = recentActivity.length - 1; i >= 0; i--) {
240233
if (
241234
recentActivity[i].type === 'tool_call' &&
242-
recentActivity[i].content === toolName &&
235+
recentActivity[i].id === callId &&
243236
recentActivity[i].status === SubagentState.RUNNING
244237
) {
245-
recentActivity[i].status = SubagentState.ERROR;
238+
recentActivity[i].status = targetStatus;
246239
updated = true;
247240
break;
248241
}

‎packages/core/src/agents/local-session-invocation.test.ts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @license
3-
* Copyright 2025 Google LLC
3+
* Copyright 2026 Google LLC
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

@@ -103,7 +103,7 @@ describe('LocalSessionInvocation', () => {
103103
});
104104

105105
describe('getDescription', () => {
106-
it('should format the description with inputs and truncate long values and overall length', () => {
106+
it('should format the description with inputs', () => {
107107
setupMockSession({});
108108
const params = { task: 'Analyze data', priority: 5 };
109109
const invocation = new LocalSessionInvocation(
@@ -118,7 +118,7 @@ describe('LocalSessionInvocation', () => {
118118
);
119119
});
120120

121-
it('should truncate long input values', () => {
121+
it('should not truncate long input values', () => {
122122
setupMockSession({});
123123
const longTask = 'A'.repeat(100);
124124
const params = { task: longTask };
@@ -130,11 +130,11 @@ describe('LocalSessionInvocation', () => {
130130
);
131131
const description = invocation.getDescription();
132132
expect(description).toBe(
133-
`Running subagent 'MockAgent' with inputs: { task: ${'A'.repeat(50)} }`,
133+
`Running subagent 'MockAgent' with inputs: { task: ${'A'.repeat(100)} }`,
134134
);
135135
});
136136

137-
it('should truncate the overall description if it exceeds the limit', () => {
137+
it('should not truncate the overall description', () => {
138138
setupMockSession({});
139139
const longNameDef: LocalAgentDefinition = {
140140
...testDefinition,
@@ -151,7 +151,7 @@ describe('LocalSessionInvocation', () => {
151151
mockMessageBus,
152152
);
153153
const description = invocation.getDescription();
154-
expect(description.length).toBe(200);
154+
expect(description.length).toBeGreaterThan(300);
155155
});
156156
});
157157

@@ -267,13 +267,13 @@ describe('LocalSessionInvocation', () => {
267267
isSubagentActivityEvent: true,
268268
agentName: 'MockAgent',
269269
type: 'TOOL_CALL_START',
270-
data: { name: 'ls', args: {} },
270+
data: { name: 'ls', args: {}, callId: 'call-123' },
271271
});
272272
capturedActivityCallback!({
273273
isSubagentActivityEvent: true,
274274
agentName: 'MockAgent',
275275
type: 'TOOL_CALL_END',
276-
data: { name: 'ls', data: {} },
276+
data: { name: 'ls', data: {}, id: 'call-123' },
277277
});
278278

279279
await executePromise;
@@ -411,7 +411,7 @@ describe('LocalSessionInvocation', () => {
411411
isSubagentActivityEvent: true,
412412
agentName: 'MockAgent',
413413
type: 'TOOL_CALL_START',
414-
data: { name: 'dangerous_tool', args: {} },
414+
data: { name: 'dangerous_tool', args: {}, callId: 'call-rej' },
415415
});
416416
capturedActivityCallback!({
417417
isSubagentActivityEvent: true,
@@ -421,6 +421,7 @@ describe('LocalSessionInvocation', () => {
421421
name: 'dangerous_tool',
422422
error: `${SUBAGENT_REJECTED_ERROR_PREFIX} Rethink approach.`,
423423
errorType: SubagentActivityErrorType.REJECTED,
424+
callId: 'call-rej',
424425
},
425426
});
426427

@@ -742,13 +743,13 @@ describe('LocalSessionInvocation', () => {
742743
isSubagentActivityEvent: true,
743744
agentName: 'MockAgent',
744745
type: 'TOOL_CALL_START',
745-
data: { name: 'failing_tool', args: {} },
746+
data: { name: 'failing_tool', args: {}, callId: 'call-err' },
746747
});
747748
capturedActivityCallback!({
748749
isSubagentActivityEvent: true,
749750
agentName: 'MockAgent',
750751
type: 'TOOL_CALL_END',
751-
data: { name: 'failing_tool', data: { isError: true } },
752+
data: { name: 'failing_tool', data: { isError: true }, id: 'call-err' },
752753
});
753754

754755
await executePromise;

‎packages/core/src/agents/local-session-invocation.ts‎

Lines changed: 36 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @license
3-
* Copyright 2025 Google LLC
3+
* Copyright 2026 Google LLC
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

@@ -35,8 +35,6 @@ import { checkExhaustive } from '../utils/checks.js';
3535
import { LocalSubagentSession } from './local-subagent-protocol.js';
3636
import type { AgentEvent } from '../agent/types.js';
3737

38-
const INPUT_PREVIEW_MAX_LENGTH = 50;
39-
const DESCRIPTION_MAX_LENGTH = 200;
4038
const MAX_RECENT_ACTIVITY = 3;
4139

4240
/** Optional configuration for subagent invocations. */
@@ -90,14 +88,10 @@ export class LocalSessionInvocation extends BaseToolInvocation<
9088
*/
9189
getDescription(): string {
9290
const inputSummary = Object.entries(this.params)
93-
.map(
94-
([key, value]) =>
95-
`${key}: ${String(value).slice(0, INPUT_PREVIEW_MAX_LENGTH)}`,
96-
)
91+
.map(([key, value]) => `${key}: ${String(value)}`)
9792
.join(', ');
9893

99-
const description = `Running subagent '${this.definition.name}' with inputs: { ${inputSummary} }`;
100-
return description.slice(0, DESCRIPTION_MAX_LENGTH);
94+
return `Running subagent '${this.definition.name}' with inputs: { ${inputSummary} }`;
10195
}
10296

10397
private publishActivity(activity: SubagentActivityItem): void {
@@ -164,9 +158,11 @@ export class LocalSessionInvocation extends BaseToolInvocation<
164158
? sanitizeErrorMessage(String(activity.data['description']))
165159
: undefined;
166160
const args = JSON.stringify(sanitizeToolArgs(activity.data['args']));
167-
// TODO: Use unique callId when available instead of randomUUID to support parallel calls reliably.
161+
const callId = activity.data['callId']
162+
? String(activity.data['callId'])
163+
: randomUUID();
168164
recentActivity.push({
169-
id: randomUUID(),
165+
id: callId,
170166
type: 'tool_call',
171167
content: name,
172168
displayName,
@@ -183,26 +179,28 @@ export class LocalSessionInvocation extends BaseToolInvocation<
183179
break;
184180
}
185181
case 'TOOL_CALL_END': {
186-
const rawName = activity.data['name'];
187-
const name = typeof rawName === 'string' ? rawName.trim() : '';
188182
const data = activity.data['data'];
189183
const isError = isToolActivityError(data);
190184

191-
// TODO: Matching tool calls by name is unreliable when parallel tool calls are active.
192-
// Use unique callId when available to find the correct TOOL_CALL_START item.
193-
for (let i = recentActivity.length - 1; i >= 0; i--) {
194-
if (
195-
recentActivity[i].type === 'tool_call' &&
196-
recentActivity[i].content === name &&
197-
recentActivity[i].status === SubagentState.RUNNING
198-
) {
199-
recentActivity[i].status = isError
200-
? SubagentState.ERROR
201-
: SubagentState.COMPLETED;
202-
updated = true;
203-
204-
this.publishActivity(recentActivity[i]);
205-
break;
185+
const callId = activity.data['id']
186+
? String(activity.data['id'])
187+
: undefined;
188+
189+
if (callId) {
190+
for (let i = recentActivity.length - 1; i >= 0; i--) {
191+
if (
192+
recentActivity[i].type === 'tool_call' &&
193+
recentActivity[i].id === callId &&
194+
recentActivity[i].status === SubagentState.RUNNING
195+
) {
196+
recentActivity[i].status = isError
197+
? SubagentState.ERROR
198+
: SubagentState.COMPLETED;
199+
updated = true;
200+
201+
this.publishActivity(recentActivity[i]);
202+
break;
203+
}
206204
}
207205
}
208206
break;
@@ -219,33 +217,23 @@ export class LocalSessionInvocation extends BaseToolInvocation<
219217
errorType === SubagentActivityErrorType.REJECTED ||
220218
error.startsWith(SUBAGENT_REJECTED_ERROR_PREFIX);
221219

222-
const toolName = activity.data['name']
223-
? String(activity.data['name'])
220+
const callId = activity.data['callId']
221+
? String(activity.data['callId'])
224222
: undefined;
225223

226-
// TODO: Matching tool calls by name is unreliable when parallel tool calls are active.
227-
// Use unique callId when available to find the correct TOOL_CALL_START item.
228-
if (toolName && (isCancellation || isRejection)) {
229-
for (let i = recentActivity.length - 1; i >= 0; i--) {
230-
if (
231-
recentActivity[i].type === 'tool_call' &&
232-
recentActivity[i].content === toolName &&
233-
recentActivity[i].status === SubagentState.RUNNING
234-
) {
235-
recentActivity[i].status = SubagentState.CANCELLED;
236-
updated = true;
237-
break;
238-
}
239-
}
240-
} else if (toolName) {
241-
// Mark non-rejection/non-cancellation errors as 'error'
224+
if (callId) {
225+
const targetStatus =
226+
isCancellation || isRejection
227+
? SubagentState.CANCELLED
228+
: SubagentState.ERROR;
229+
242230
for (let i = recentActivity.length - 1; i >= 0; i--) {
243231
if (
244232
recentActivity[i].type === 'tool_call' &&
245-
recentActivity[i].content === toolName &&
233+
recentActivity[i].id === callId &&
246234
recentActivity[i].status === SubagentState.RUNNING
247235
) {
248-
recentActivity[i].status = SubagentState.ERROR;
236+
recentActivity[i].status = targetStatus;
249237
updated = true;
250238
break;
251239
}

0 commit comments

Comments
 (0)