Fix handoff widget visibility: derive from response mode, persist modeInfo (#298867)

* Fix handoff widget visibility: derive from response mode, persist modeInfo, fix lifecycle

* Fix handoff widget response mode logic

* Make findModeByName search _customModeInstances directly like findModeById does, bypassing the getCustomModes() gate

* Fix tests

* Fix custom mode lookup to avoid builtin name collisions

* address review: use findModeByName consistently, respect getCustomModes guard
This commit is contained in:
Harald Kirschner
2026-03-09 02:42:55 -07:00
committed by GitHub
parent 1f3de37bc2
commit acd63e770b
8 changed files with 79 additions and 15 deletions

View File

@@ -53,7 +53,7 @@ import { ChatContextKeys } from '../../common/actions/chatContextKeys.js';
import { applyingChatEditsFailedContextKey, decidedChatEditingResourceContextKey, hasAppliedChatEditsContextKey, hasUndecidedChatEditingResourceContextKey, IChatEditingService, IChatEditingSession, inChatEditingSessionContextKey, ModifiedFileEntryState } from '../../common/editing/chatEditingService.js';
import { IChatLayoutService } from '../../common/widget/chatLayoutService.js';
import { IChatModel, IChatModelInputState, IChatResponseModel } from '../../common/model/chatModel.js';
import { ChatMode, getModeNameForTelemetry, IChatModeService } from '../../common/chatModes.js';
import { ChatMode, getModeNameForTelemetry, IChatMode, IChatModeService } from '../../common/chatModes.js';
import { chatAgentLeader, ChatRequestAgentPart, ChatRequestDynamicVariablePart, ChatRequestSlashPromptPart, ChatRequestToolPart, ChatRequestToolSetPart, chatSubcommandLeader, formatChatQuestion, IParsedChatRequest } from '../../common/requestParser/chatParserTypes.js';
import { ChatRequestParser } from '../../common/requestParser/chatRequestParser.js';
import { getDynamicVariablesForWidget, getSelectedToolAndToolSetsForWidget } from '../attachments/chatVariables.js';
@@ -1182,27 +1182,34 @@ export class ChatWidget extends Disposable implements IChatWidget {
const lastItem = items[items.length - 1];
const lastResponseComplete = lastItem && isResponseVM(lastItem) && lastItem.isComplete;
if (!lastResponseComplete) {
if (!lastResponseComplete || lastItem.isCanceled) {
this.chatSuggestNextWidget.hide();
return;
}
// Get the currently selected mode directly from the observable
// Note: We use currentModeObs instead of currentModeKind because currentModeKind returns
// the ChatModeKind enum (e.g., 'agent'), which doesn't distinguish between custom modes.
// Custom modes all have kind='agent' but different IDs.
const currentMode = this.input.currentModeObs.get();
const handoffs = currentMode?.handOffs?.get();
// Only show if: mode has handoffs AND chat has content AND not quick chat
const shouldShow = currentMode && handoffs && handoffs.length > 0;
// Derive handoffs from the mode that generated the last response, not the current UI selection.
// This ensures handoffs reflect what the response agent offers, regardless of mode picker state.
// Fall back to the current mode picker for old sessions where modeInfo was not persisted.
const modeInfo = lastItem.model.request?.modeInfo;
let responseMode: IChatMode | undefined;
if (modeInfo?.modeInstructions?.name) {
responseMode = this.chatModeService.findModeByName(modeInfo.modeInstructions.name);
} else if (modeInfo?.modeId) {
responseMode = this.chatModeService.findModeById(modeInfo.modeId);
} else {
responseMode = this.input.currentModeObs.get();
}
if (shouldShow) {
const handoffs = responseMode?.handOffs?.get();
if (responseMode && handoffs && handoffs.length > 0) {
// Log telemetry only when widget transitions from hidden to visible
const wasHidden = this.chatSuggestNextWidget.domNode.style.display === 'none';
this.chatSuggestNextWidget.render(currentMode);
this.chatSuggestNextWidget.render(responseMode);
if (wasHidden) {
this.telemetryService.publicLog2<ChatHandoffWidgetShownEvent, ChatHandoffWidgetShownClassification>('chat.handoffWidgetShown', {
agent: getModeNameForTelemetry(currentMode),
agent: getModeNameForTelemetry(responseMode),
handoffCount: handoffs.length
});
}
@@ -2006,6 +2013,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
if (e.kind === 'addRequest') {
this.inputPart.clearTodoListWidget(this.viewModel?.sessionResource, false);
this._sessionIsEmptyContextKey.set(false);
this.chatSuggestNextWidget.hide();
}
// Hide widget on request removal
if (e.kind === 'removeRequest') {
@@ -2036,6 +2044,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
this.listWidget.scrollToEnd();
}
this.renderChatSuggestNextWidget();
this.updateChatInputContext();
this.input.renderChatTodoListWidget(this.viewModel.sessionResource);
}

View File

@@ -1449,6 +1449,7 @@ export interface ISerializableChatRequestData extends ISerializableChatResponseD
confirmation?: string;
editedFileEvents?: IChatAgentEditedFileEvent[];
modelId?: string;
modeInfo?: IChatRequestModeInfo;
}
export interface ISerializableMarkdownInfo {
@@ -2315,6 +2316,7 @@ export class ChatModel extends Disposable implements IChatModel {
confirmation: raw.confirmation,
editedFileEvents: raw.editedFileEvents,
modelId: raw.modelId,
modeInfo: raw.modeInfo,
});
request.shouldBeRemovedOnSend = raw.isHidden ? { requestId: raw.requestId } : raw.shouldBeRemovedOnSend;
// eslint-disable-next-line @typescript-eslint/no-explicit-any, local/code-no-any-casts
@@ -2657,6 +2659,7 @@ export class ChatModel extends Disposable implements IChatModel {
confirmation: r.confirmation,
editedFileEvents: r.editedFileEvents,
modelId: r.modelId,
modeInfo: r.modeInfo,
...r.response?.toJSON(),
};
}),

View File

@@ -148,6 +148,7 @@ const requestSchema = Adapt.object<IChatRequestModel, ISerializableChatRequestDa
contentReferences: Adapt.v(m => m.response?.contentReferences, objectsEqual),
codeCitations: Adapt.v(m => m.response?.codeCitations, objectsEqual),
timeSpentWaiting: Adapt.v(m => m.response?.timestamp), // based on response timestamp
modeInfo: Adapt.v(m => m.modeInfo, objectsEqual),
}, {
sealed: (o) => o.modelState?.value === ResponseModelState.Cancelled || o.modelState?.value === ResponseModelState.Failed || o.modelState?.value === ResponseModelState.Complete,
});

View File

@@ -75,6 +75,7 @@
confirmation: undefined,
editedFileEvents: undefined,
modelId: undefined,
modeInfo: undefined,
responseId: undefined,
result: { metadata: { metadataKey: "value" } },
responseMarkdownInfo: undefined,

View File

@@ -75,6 +75,7 @@
confirmation: undefined,
editedFileEvents: undefined,
modelId: undefined,
modeInfo: undefined,
responseId: undefined,
result: { errorDetails: { message: "No activated agent with id \"ChatProviderWithUsedContext\"" } },
responseMarkdownInfo: undefined,

View File

@@ -77,6 +77,7 @@
confirmation: undefined,
editedFileEvents: undefined,
modelId: undefined,
modeInfo: undefined,
responseId: undefined,
result: { metadata: { metadataKey: "value" } },
responseMarkdownInfo: undefined,
@@ -162,6 +163,7 @@
confirmation: undefined,
editedFileEvents: undefined,
modelId: undefined,
modeInfo: undefined,
responseId: undefined,
result: { },
responseMarkdownInfo: undefined,

View File

@@ -77,6 +77,7 @@
confirmation: undefined,
editedFileEvents: undefined,
modelId: undefined,
modeInfo: undefined,
responseId: undefined,
result: { errorDetails: { message: "No activated agent with id \"ChatProviderWithUsedContext\"" } },
responseMarkdownInfo: undefined,

View File

@@ -25,10 +25,10 @@ import { TestExtensionService, TestStorageService } from '../../../../../test/co
import { CellUri } from '../../../../notebook/common/notebookCommon.js';
import { IChatRequestImplicitVariableEntry, IChatRequestStringVariableEntry, IChatRequestFileEntry, StringChatContextValue } from '../../../common/attachments/chatVariableEntries.js';
import { ChatAgentService, IChatAgentService } from '../../../common/participants/chatAgents.js';
import { ChatModel, ChatRequestModel, IExportableChatData, ISerializableChatData1, ISerializableChatData2, ISerializableChatData3, isExportableSessionData, isSerializableSessionData, normalizeSerializableChatData, Response } from '../../../common/model/chatModel.js';
import { ChatModel, ChatRequestModel, IChatRequestModeInfo, IExportableChatData, ISerializableChatData1, ISerializableChatData2, ISerializableChatData3, isExportableSessionData, isSerializableSessionData, normalizeSerializableChatData, Response } from '../../../common/model/chatModel.js';
import { ChatRequestTextPart } from '../../../common/requestParser/chatParserTypes.js';
import { ChatRequestQueueKind, IChatService, IChatToolInvocation } from '../../../common/chatService/chatService.js';
import { ChatAgentLocation } from '../../../common/constants.js';
import { ChatAgentLocation, ChatModeKind } from '../../../common/constants.js';
import { MockChatService } from '../chatService/mockChatService.js';
suite('ChatModel', () => {
@@ -274,6 +274,52 @@ suite('ChatModel', () => {
// Should keep file attachments and implicit attachments with URI values
assert.deepStrictEqual(serialized.attachments, [fileAttachment, implicitWithUri]);
});
test('modeInfo roundtrips through serialization', async () => {
const modeInfo: IChatRequestModeInfo = {
kind: ChatModeKind.Agent,
isBuiltin: false,
modeId: 'custom',
modeInstructions: {
name: 'plan',
content: 'You are a planning agent',
toolReferences: [],
},
applyCodeBlockSuggestionId: undefined,
};
const serializableData: ISerializableChatData3 = {
version: 3,
sessionId: 'test-modeinfo-session',
creationDate: Date.now(),
customTitle: undefined,
initialLocation: ChatAgentLocation.Chat,
responderUsername: 'bot',
requests: [{
requestId: 'req1',
message: { text: 'plan something', parts: [] },
variableData: { variables: [] },
response: [{ value: 'Here is my plan', isTrusted: false }],
modelState: { value: 1 /* ResponseModelState.Complete */, completedAt: Date.now() },
modeInfo,
}],
};
const model = testDisposables.add(instantiationService.createInstance(
ChatModel,
{ value: serializableData, serializer: undefined! },
{ initialLocation: ChatAgentLocation.Chat, canUseTools: true }
));
const requests = model.getRequests();
assert.strictEqual(requests.length, 1);
assert.deepStrictEqual(requests[0].modeInfo, modeInfo);
// Verify roundtrip through toExport
const exported = model.toExport();
assert.strictEqual(exported.requests.length, 1);
assert.deepStrictEqual(exported.requests[0].modeInfo, modeInfo);
});
});
suite('Response', () => {