Skip to content

Commit 120066c

Browse files
authored
agentHost: adopt activity events and fix client tool stall in subagent (#313603)
* agentHost: adopt activity events and fix client tool stall in subagent * comments
1 parent 4d75b7a commit 120066c

15 files changed

Lines changed: 472 additions & 74 deletions

File tree

src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC
300300
} : {}),
301301
summary: s.title,
302302
status: s.status,
303+
activity: s.activity,
303304
workingDirectory: typeof s.workingDirectory === 'string' ? toAgentHostUri(URI.parse(s.workingDirectory), this._connectionAuthority) : undefined,
304305
isRead: !!(s.status & SessionStatus.IsRead),
305306
isArchived: !!(s.status & SessionStatus.IsArchived),

src/vs/platform/agentHost/common/agentService.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ export interface IAgentSessionMetadata {
8282
readonly project?: IAgentSessionProjectInfo;
8383
readonly summary?: string;
8484
readonly status?: SessionStatus;
85+
/** Human-readable description of what the session is currently doing. */
86+
readonly activity?: string;
8587
readonly model?: ModelSelection;
8688
readonly workingDirectory?: URI;
8789
readonly customizationDirectory?: URI;
@@ -268,6 +270,14 @@ export interface IAgentToolPendingConfirmationSignal {
268270
readonly permissionKind?: 'shell' | 'write' | 'mcp' | 'read' | 'url' | 'custom-tool' | 'hook' | 'memory';
269271
/** Host-only auto-approval path target (not part of the dispatched action). */
270272
readonly permissionPath?: string;
273+
/**
274+
* If set, the tool call belongs to the subagent rooted at this
275+
* parent tool call. Used by the host to route the resulting
276+
* `SessionToolCallReady` to the subagent session — otherwise the
277+
* action would land on the parent session, where there is no
278+
* matching `SessionToolCallStart`.
279+
*/
280+
readonly parentToolCallId?: string;
271281
}
272282

273283
/**

src/vs/platform/agentHost/node/agentHostStateManager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ export class AgentHostStateManager extends Disposable {
334334
const changes: Partial<SessionSummary> = {};
335335
if (current.title !== lastNotified.title) { changes.title = current.title; }
336336
if (current.status !== lastNotified.status) { changes.status = current.status; }
337+
if (current.activity !== lastNotified.activity) { changes.activity = current.activity; }
337338
if (current.modifiedAt !== lastNotified.modifiedAt) { changes.modifiedAt = current.modifiedAt; }
338339
if (current.project !== lastNotified.project) { changes.project = current.project; }
339340
if (current.model !== lastNotified.model) { changes.model = current.model; }

src/vs/platform/agentHost/node/agentService.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ export class AgentService extends Disposable implements IAgentService {
202202
...s,
203203
summary: liveState.summary.title || s.summary,
204204
status: liveState.summary.status,
205+
activity: liveState.summary.activity,
205206
model: liveState.summary.model ?? s.model,
206207
};
207208
}

src/vs/platform/agentHost/node/agentSideEffects.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,15 @@ export class AgentSideEffects extends Disposable {
274274
}
275275

276276
// Route signals with parentToolCallId to the subagent session.
277-
const parentToolCallId = signal.kind === 'action' ? signal.parentToolCallId : undefined;
277+
// Both action signals and pending_confirmation signals can carry
278+
// a parentToolCallId — for client tools inside a subagent the
279+
// permission flow fires `pending_confirmation` for an inner tool
280+
// call, and that signal must be routed to the subagent session
281+
// (otherwise the resulting SessionToolCallReady would land on the
282+
// parent session, which has no matching SessionToolCallStart).
283+
const parentToolCallId = signal.kind === 'action' || signal.kind === 'pending_confirmation'
284+
? signal.parentToolCallId
285+
: undefined;
278286
if (parentToolCallId) {
279287
const subagentKey = `${sessionKey}:${parentToolCallId}`;
280288
const subagentSession = this._subagentSessions.get(subagentKey);
@@ -285,7 +293,7 @@ export class AgentSideEffects extends Disposable {
285293
}
286294
const subTurnId = this._stateManager.getActiveTurnId(subagentSession);
287295
if (subTurnId) {
288-
this._dispatchActionForSession(signal, subagentSession, subTurnId);
296+
this._dispatchActionForSession(signal, subagentSession, subTurnId, agent);
289297
}
290298
return;
291299
}
@@ -303,8 +311,9 @@ export class AgentSideEffects extends Disposable {
303311
}
304312

305313
// Route pending_confirmation signals for tools inside subagent sessions
306-
// (the signal lacks parentToolCallId, but the tool was previously
307-
// registered under its subagent session key in _toolCallAgents).
314+
// (legacy path for signals without an explicit parentToolCallId — the
315+
// tool was previously registered under its subagent session key in
316+
// _toolCallAgents).
308317
if (signal.kind === 'pending_confirmation') {
309318
const subagentSession = this._findSubagentSessionForToolCall(sessionKey, signal.state.toolCallId);
310319
if (subagentSession) {

src/vs/platform/agentHost/node/copilot/copilotAgent.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,8 @@ export class CopilotAgent extends Disposable implements IAgent {
769769
}
770770

771771
onClientToolCallComplete(session: URI, toolCallId: string, result: ToolCallResult): void {
772-
const entry = this._sessions.get(AgentSession.id(session));
772+
const sessionId = AgentSession.id(parseSubagentSessionUri(session.toString())?.parentSession || session);
773+
const entry = this._sessions.get(sessionId);
773774
entry?.handleClientToolCallComplete(toolCallId, result);
774775
}
775776

src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ export class CopilotAgentSession extends Disposable {
150150
readonly sessionUri: URI;
151151

152152
/** Tracks active tool invocations so we can produce past-tense messages on completion. */
153-
private readonly _activeToolCalls = new Map<string, { toolName: string; displayName: string; parameters: Record<string, unknown> | undefined; content: ToolResultContent[] }>();
153+
private readonly _activeToolCalls = new Map<string, { toolName: string; displayName: string; parameters: Record<string, unknown> | undefined; content: ToolResultContent[]; parentToolCallId: string | undefined }>();
154154
/** Pending permission requests awaiting a renderer-side decision. */
155155
private readonly _pendingPermissions = new Map<string, DeferredPromise<boolean>>();
156156
/** Pending user input requests awaiting a renderer-side answer. */
@@ -206,6 +206,8 @@ export class CopilotAgentSession extends Disposable {
206206
private _currentMarkdownPartId: string | undefined;
207207
/** Current reasoning response part ID for the active turn. Reset on each new turn. */
208208
private _currentReasoningPartId: string | undefined;
209+
/** Tracks whether a non-empty activity has been published, so we only emit a clear when needed. */
210+
private _hasReportedActivity = false;
209211

210212
constructor(
211213
options: ICopilotAgentSessionOptions,
@@ -612,6 +614,11 @@ export class CopilotAgentSession extends Disposable {
612614

613615
// Fire a pending_confirmation signal to transition the tool to PendingConfirmation
614616
const toolName = request.toolName ?? request.kind;
617+
// Forward the tool's parentToolCallId (if any) so the host can
618+
// route the resulting SessionToolCallReady to the correct
619+
// subagent session — without it the action would land on the
620+
// parent session, which has no matching SessionToolCallStart.
621+
const parentToolCallId = this._activeToolCalls.get(toolCallId)?.parentToolCallId;
615622
this._onDidSessionProgress.fire({
616623
kind: 'pending_confirmation',
617624
session: this.sessionUri,
@@ -627,6 +634,7 @@ export class CopilotAgentSession extends Disposable {
627634
},
628635
permissionKind,
629636
permissionPath,
637+
parentToolCallId,
630638
});
631639

632640
const approved = await deferred.p;
@@ -992,6 +1000,21 @@ export class CopilotAgentSession extends Disposable {
9921000
this._register(wrapper.onToolStart(e => {
9931001
if (isHiddenTool(e.data.toolName)) {
9941002
this._logService.trace(`[Copilot:${sessionId}] Tool started (hidden): ${e.data.toolName}`);
1003+
// The CLI uses the `report_intent` tool to signal what the
1004+
// agent is currently doing. Surface this as session activity
1005+
// so the UI can show a live "what is the agent doing now?"
1006+
// hint while the turn is in progress.
1007+
if (e.data.toolName === 'report_intent') {
1008+
const intent = (e.data.arguments as { intent?: unknown } | undefined)?.intent;
1009+
if (typeof intent === 'string' && intent.length > 0) {
1010+
this._hasReportedActivity = true;
1011+
this._emitAction({
1012+
type: ActionType.SessionActivityChanged,
1013+
session: this._protocolSession(),
1014+
activity: intent,
1015+
});
1016+
}
1017+
}
9951018
return;
9961019
}
9971020
this._logService.info(`[Copilot:${sessionId}] Tool started: ${e.data.toolName}`);
@@ -1007,7 +1030,7 @@ export class CopilotAgentSession extends Disposable {
10071030
toolArgs = tryStringify(parameters);
10081031
}
10091032
const displayName = getToolDisplayName(e.data.toolName);
1010-
this._activeToolCalls.set(e.data.toolCallId, { toolName: e.data.toolName, displayName, parameters, content: [] });
1033+
this._activeToolCalls.set(e.data.toolCallId, { toolName: e.data.toolName, displayName, parameters, content: [], parentToolCallId: e.data.parentToolCallId });
10111034
const toolKind = getToolKind(e.data.toolName);
10121035
const subagentMeta = toolKind === 'subagent' ? getSubagentMetadata(parameters) : undefined;
10131036
const toolClientId = this._clientToolNames.has(e.data.toolName) ? this._appliedSnapshot.clientId : undefined;
@@ -1124,6 +1147,17 @@ export class CopilotAgentSession extends Disposable {
11241147

11251148
this._register(wrapper.onIdle(() => {
11261149
this._logService.info(`[Copilot:${sessionId}] Session idle`);
1150+
// Clear any in-progress activity description set during the
1151+
// turn (e.g. via the `report_intent` tool) — the agent is no
1152+
// longer doing anything once the turn completes.
1153+
if (this._hasReportedActivity) {
1154+
this._hasReportedActivity = false;
1155+
this._emitAction({
1156+
type: ActionType.SessionActivityChanged,
1157+
session: this._protocolSession(),
1158+
activity: undefined,
1159+
});
1160+
}
11271161
this._emitAction({
11281162
type: ActionType.SessionTurnComplete,
11291163
session: this._protocolSession(),

src/vs/platform/agentHost/node/protocolServerHandler.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,7 @@ export class ProtocolServerHandler extends Disposable {
509509
provider,
510510
title: s.summary ?? 'Session',
511511
status,
512+
activity: s.activity,
512513
createdAt: s.startTime,
513514
modifiedAt: s.modifiedTime,
514515
...(s.project ? { project: { uri: s.project.uri.toString(), displayName: s.project.displayName } } : {}),

src/vs/platform/agentHost/test/node/agentSideEffects.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,83 @@ suite('AgentSideEffects', () => {
10691069
assert.strictEqual(part?.kind === ResponsePartKind.ToolCall ? part.toolCall.status : undefined, ToolCallStatus.PendingConfirmation,
10701070
'tool call should advance to PendingConfirmation for permission-gated tool_ready');
10711071
});
1072+
1073+
test('pending_confirmation for a tool inside a subagent routes to the subagent session', () => {
1074+
// Regression: a `pending_confirmation` signal for a client tool
1075+
// inside a subagent must dispatch SessionToolCallReady against
1076+
// the subagent session, not the parent. Otherwise the parent
1077+
// session sees a stray `session/toolCallReady` with no
1078+
// preceding `session/toolCallStart`, which is illegal.
1079+
setupSession();
1080+
startTurn('turn-1');
1081+
disposables.add(sideEffects.registerProgressListener(agent));
1082+
1083+
// Parent tool that delegates to a subagent.
1084+
agent.fireProgress({
1085+
kind: 'action', session: sessionUri,
1086+
action: {
1087+
type: ActionType.SessionToolCallStart, session: sessionUri.toString(), turnId: 'turn-1',
1088+
toolCallId: 'tc-parent', toolName: 'runSubagent', displayName: 'Run Subagent', toolClientId: undefined,
1089+
_meta: { toolKind: undefined, language: undefined },
1090+
},
1091+
});
1092+
agent.fireProgress({
1093+
kind: 'action', session: sessionUri,
1094+
action: {
1095+
type: ActionType.SessionToolCallReady, session: sessionUri.toString(), turnId: 'turn-1',
1096+
toolCallId: 'tc-parent', invocationMessage: 'Delegating...', toolInput: undefined,
1097+
confirmed: ToolCallConfirmationReason.NotNeeded,
1098+
},
1099+
});
1100+
agent.fireProgress({ kind: 'subagent_started', session: sessionUri, toolCallId: 'tc-parent', agentName: 'helper', agentDisplayName: 'Helper' });
1101+
1102+
// Inner client tool starts inside the subagent.
1103+
agent.fireProgress({
1104+
kind: 'action', session: sessionUri, parentToolCallId: 'tc-parent',
1105+
action: {
1106+
type: ActionType.SessionToolCallStart, session: sessionUri.toString(), turnId: 'turn-1',
1107+
toolCallId: 'tc-inner', toolName: 'problems', displayName: 'Problems', toolClientId: 'client-tools',
1108+
_meta: { toolKind: undefined, language: undefined },
1109+
},
1110+
});
1111+
1112+
// Permission flow fires `pending_confirmation` for the inner
1113+
// client tool. The signal must be routed to the subagent
1114+
// session — not to the parent — even though the signal carries
1115+
// only the parent session URI.
1116+
agent.fireProgress({
1117+
kind: 'pending_confirmation', session: sessionUri, parentToolCallId: 'tc-parent',
1118+
state: {
1119+
status: ToolCallStatus.PendingConfirmation,
1120+
toolCallId: 'tc-inner', toolName: 'problems', displayName: 'Problems',
1121+
invocationMessage: 'Get problems', toolInput: '{}',
1122+
confirmationTitle: undefined, edits: undefined,
1123+
},
1124+
permissionKind: 'custom-tool', permissionPath: undefined,
1125+
});
1126+
1127+
// The subagent session must contain the SessionToolCallReady.
1128+
const subagentUri = buildSubagentSessionUri(sessionUri.toString(), 'tc-parent');
1129+
const subState = stateManager.getSessionState(subagentUri);
1130+
const innerPart = subState?.activeTurn?.responseParts.find(
1131+
rp => rp.kind === ResponsePartKind.ToolCall && rp.toolCall.toolCallId === 'tc-inner'
1132+
);
1133+
assert.ok(innerPart, 'inner client tool call should exist on subagent session');
1134+
assert.strictEqual(
1135+
innerPart!.kind === ResponsePartKind.ToolCall ? innerPart.toolCall.status : undefined,
1136+
ToolCallStatus.Running,
1137+
'inner client tool call should advance to Running after pending_confirmation'
1138+
);
1139+
1140+
// The parent session must NOT have a stray tool call for the
1141+
// inner toolCallId — that would be a SessionToolCallReady
1142+
// without a matching SessionToolCallStart.
1143+
const parentState = stateManager.getSessionState(sessionUri.toString());
1144+
const parentInner = parentState?.activeTurn?.responseParts.find(
1145+
rp => rp.kind === ResponsePartKind.ToolCall && rp.toolCall.toolCallId === 'tc-inner'
1146+
);
1147+
assert.strictEqual(parentInner, undefined, 'parent session must not contain the inner tool call');
1148+
});
10721149
});
10731150

10741151
// ---- Session-level auto-approve (config) ----------------------------

src/vs/platform/agentHost/test/node/copilotAgent.test.ts

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { IAgentPluginManager, ISyncedCustomization } from '../../common/agentPlu
2121
import { AgentSession, type AgentSignal, type IAgentActionSignal, type IAgentSessionMetadata } from '../../common/agentService.js';
2222
import { ISessionDataService } from '../../common/sessionDataService.js';
2323
import { AHP_AUTH_REQUIRED, ProtocolError } from '../../common/state/sessionProtocol.js';
24-
import { ResponsePartKind, SessionCustomization, TurnState, type CustomizationRef, type MarkdownResponsePart, type Turn } from '../../common/state/sessionState.js';
24+
import { buildSubagentSessionUri, ResponsePartKind, SessionCustomization, TurnState, type CustomizationRef, type MarkdownResponsePart, type ToolCallResult, type Turn } from '../../common/state/sessionState.js';
2525
import { ActionType, type IDeltaAction } from '../../common/state/sessionActions.js';
2626

2727
import { AgentConfigurationService, IAgentConfigurationService } from '../../node/agentConfigurationService.js';
@@ -500,6 +500,74 @@ suite('CopilotAgent', () => {
500500
});
501501
});
502502

503+
suite('onClientToolCallComplete', () => {
504+
505+
/**
506+
* Injects a stub session into the agent's `_sessions` map so we can
507+
* observe how `onClientToolCallComplete` resolves URIs to session
508+
* entries without standing up a full Copilot SDK session.
509+
*/
510+
function installStubSession(agent: CopilotAgent, sessionId: string): { calls: { toolCallId: string; result: ToolCallResult }[] } {
511+
const calls: { toolCallId: string; result: ToolCallResult }[] = [];
512+
const stub = {
513+
handleClientToolCallComplete(toolCallId: string, result: ToolCallResult) {
514+
calls.push({ toolCallId, result });
515+
},
516+
dispose() { },
517+
};
518+
const sessions = (agent as unknown as { _sessions: Map<string, unknown> })._sessions;
519+
sessions.set(sessionId, stub);
520+
return { calls };
521+
}
522+
523+
test('routes a top-level session URI to its session entry', async () => {
524+
const agent = createTestAgent(disposables);
525+
try {
526+
const sessionUri = AgentSession.uri('copilotcli', 'session-top');
527+
const { calls } = installStubSession(agent, AgentSession.id(sessionUri));
528+
529+
const result: ToolCallResult = { success: true, pastTenseMessage: 'did it' };
530+
agent.onClientToolCallComplete(sessionUri, 'tc-top', result);
531+
532+
assert.deepStrictEqual(calls, [{ toolCallId: 'tc-top', result }]);
533+
} finally {
534+
await disposeAgent(agent);
535+
}
536+
});
537+
538+
test('routes a subagent session URI to its parent session entry', async () => {
539+
// Regression: client-tool completions for tools running inside a
540+
// subagent are dispatched against the subagent session URI by
541+
// the renderer. The agent must resolve that to the parent
542+
// session entry — only the parent owns the SDK session and the
543+
// pending deferred for the tool call.
544+
const agent = createTestAgent(disposables);
545+
try {
546+
const parentUri = AgentSession.uri('copilotcli', 'session-parent');
547+
const { calls } = installStubSession(agent, AgentSession.id(parentUri));
548+
549+
const subagentUri = URI.parse(buildSubagentSessionUri(parentUri.toString(), 'tc-parent'));
550+
const result: ToolCallResult = { success: true, pastTenseMessage: 'subagent tool done' };
551+
agent.onClientToolCallComplete(subagentUri, 'tc-inner', result);
552+
553+
assert.deepStrictEqual(calls, [{ toolCallId: 'tc-inner', result }]);
554+
} finally {
555+
await disposeAgent(agent);
556+
}
557+
});
558+
559+
test('is a no-op when no session entry exists for the resolved id', async () => {
560+
const agent = createTestAgent(disposables);
561+
try {
562+
const sessionUri = AgentSession.uri('copilotcli', 'session-missing');
563+
// No stub installed — the call should be silently ignored.
564+
agent.onClientToolCallComplete(sessionUri, 'tc-x', { success: true, pastTenseMessage: 'noop' });
565+
} finally {
566+
await disposeAgent(agent);
567+
}
568+
});
569+
});
570+
503571
suite('worktree announcement', () => {
504572
// Drives a real session through worktree creation (calling the
505573
// agent's _resolveSessionWorkingDirectory via a test seam so we don't

0 commit comments

Comments
 (0)