From 4ad09abc833845f1e37371dc48ddfbd240a1c205 Mon Sep 17 00:00:00 2001 From: Hazem Ali Date: Sun, 15 Nov 2026 20:33:37 +0200 Subject: [PATCH] Fix parent chat state after unmapped subagent errors --- src/vs/platform/agentHost/AGENTS.md | 2 + .../node/copilot/copilotAgentSession.ts | 11 +-- .../test/node/copilotAgentSession.test.ts | 81 ++++++++++++------- 3 files changed, 58 insertions(+), 36 deletions(-) diff --git a/src/vs/platform/agentHost/AGENTS.md b/src/vs/platform/agentHost/AGENTS.md index a73b9a9bf6ec5b..f8ec32115f4745 100644 --- a/src/vs/platform/agentHost/AGENTS.md +++ b/src/vs/platform/agentHost/AGENTS.md @@ -680,6 +680,8 @@ Each additional chat is backed by a fresh top-level SDK session (`sdkSessionId = ### Copilot (`node/copilot/copilotAgent.ts`) +SDK `session.error` events with an `agentId` belong to that child. Route them only through its tracked tool-call identity; if the child is unmapped, retain diagnostics without emitting a root chat error or ending the root turn. Child failures do not finalize root tool-call accounting. + `AgentConfigurationService.copilotM365EnabledAtStartup` captures the validated `m365Enabled` root value once after loading persisted configuration. Only `true` enables it; missing or invalid values leave it off. The root value remains live requested configuration, while the effective value stays fixed for that Agent Host lifetime. A new host reads the persisted request; restarting VS Code recreates the local host. Reloading a window, connecting another client, or restarting only the Copilot SDK runtime does not recapture the value in an existing host. This is a host-lifetime snapshot, not a separate application-lifetime latch, and does not initiate account discovery, choose a service or migrate history. Copilot also has no AH-session container: diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index e503dbacf3bc90..62caadc09665d9 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -11988,13 +11988,14 @@ export class CopilotAgentSession extends Disposable { } else { reportError(); } + if (this._shouldDropUnmappedSubagentEvent(e, 'session.error')) { + return; + } const parentToolCallId = this._parentToolCallIdForSubagentEvent(e); - if (turn) { + if (turn && !parentToolCallId) { this._reportToolCallDetails(turn, 'failed'); - if (!parentToolCallId) { - turn.mcpUsage?.complete('failed'); - this._attributeLocalModelTurn(turn.id); - } + turn.mcpUsage?.complete('failed'); + this._attributeLocalModelTurn(turn.id); } this._emitAction({ type: ActionType.ChatError, diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index 32228a4bb41352..85cf9de93a2456 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -27078,7 +27078,55 @@ Use the attached image as context. }]); }); - test('unmapped subagent error event is forwarded as non-resumable', async () => { + test('root quota error still ends the parent turn', async () => { + const { session, mockSession, signals } = await createAgentSession(disposables); + session.resetTurnState('root-turn'); + mockSession.fire('session.error', { + errorType: 'quota', message: 'Copilot premium request allowance exhausted', statusCode: 402, + }); + + assert.deepStrictEqual({ + hasActiveTurn: session.hasActiveTurn, + errors: signals.flatMap(signal => isAction(signal, ActionType.ChatError) ? [signal.parentToolCallId] : []), + }, { hasActiveTurn: false, errors: [undefined] }); + }); + + for (const mapped of [false, true]) { + test(`subagent quota error preserves the running parent turn (mapped=${mapped})`, async () => { + const { session, mockSession, signals } = await createAgentSession(disposables); + session.resetTurnState('byok-parent'); + mockSession.fire('assistant.turn_start', { turnId: 'sdk-parent' }); + if (mapped) { + mockSession.fire('subagent.started', { + toolCallId: 'child-task', agentName: 'explore', agentDisplayName: 'Explore', agentDescription: 'Explore tests', + }, { agentId: 'child-agent' }); + } + signals.length = 0; + + mockSession.fire('session.error', { + errorType: 'quota', message: 'Copilot premium request allowance exhausted', statusCode: 402, + }, { agentId: 'child-agent' }); + const activeAfterChildError = session.hasActiveTurn; + mockSession.fire('assistant.message_delta', { messageId: 'parent-message', deltaContent: 'Still working' }); + mockSession.fire('session.idle', {}); + + assert.deepStrictEqual({ + activeAfterChildError, + errors: signals.flatMap(signal => isAction(signal, ActionType.ChatError) ? [signal.parentToolCallId] : []), + parentOutput: getActions(signals).flatMap(action => action.type === ActionType.ChatResponsePart && action.part.kind === ResponsePartKind.Markdown ? [action.part.content] : []), + completed: getActions(signals).flatMap(action => action.type === ActionType.ChatTurnComplete ? [action.turnId] : []), + hasActiveTurn: session.hasActiveTurn, + }, { + activeAfterChildError: true, + errors: mapped ? ['child-task'] : [], + parentOutput: ['Still working'], + completed: ['byok-parent'], + hasActiveTurn: false, + }); + }); + } + + test('unmapped subagent error is logged without failing the parent turn', async () => { const telemetryService = new CapturingTelemetryService(); const { session, mockSession, signals } = await createAgentSession(disposables, { telemetryService }); session.resetTurnState('turn-1'); @@ -27092,36 +27140,7 @@ Use the attached image as context. serviceRequestId: 'service-request-id', } as SessionEventPayload<'session.error'>['data'], { id: 'session-error-event', parentId: 'previous-event', agentId: 'sdk-agent-id' }); - assert.strictEqual(signals.length, 1); - assert.ok(isAction(signals[0], ActionType.ChatError)); - if (isAction(signals[0], ActionType.ChatError)) { - const action = signals[0].action as ChatErrorAction; - assert.deepStrictEqual({ - error: action.part.error, - resumable: action.part.resumable, - }, { - error: { - errorType: 'TestError', - message: 'something went wrong', - stack: 'Error: something went wrong', - _meta: { - chatError: { - fetchError: { - type: 'failed', - reason: 'something went wrong', - requestId: 'provider-request-id', - serverRequestId: 'service-request-id', - capiError: { - code: 'test-code', - message: 'something went wrong', - }, - }, - }, - }, - }, - resumable: undefined, - }); - } + assert.deepStrictEqual({ actions: getActions(signals), hasActiveTurn: session.hasActiveTurn }, { actions: [], hasActiveTurn: true }); assert.deepStrictEqual(telemetryService.events.filter(event => event.eventName === 'agentHost.copilotSdkSessionError'), [{ eventName: 'agentHost.copilotSdkSessionError', data: {