Skip to content

Commit 9b8ae15

Browse files
authored
[release/1.118] On agent mode change, ignore stateful marker (#313071)
* on agent mode change, ignore stateful marker * updates to modechange
1 parent fa7dd79 commit 9b8ae15

10 files changed

Lines changed: 336 additions & 6 deletions

File tree

extensions/copilot/src/extension/agents/vscode-node/test/planAgentProvider.spec.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { DisposableStore } from '../../../../util/vs/base/common/lifecycle';
1818
import { SyncDescriptor } from '../../../../util/vs/platform/instantiation/common/descriptors';
1919
import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation';
2020
import { createExtensionUnitTestingServices } from '../../../test/node/services';
21-
import { buildAgentMarkdown } from '../agentTypes';
21+
import { buildAgentMarkdown, DEFAULT_READ_TOOLS } from '../agentTypes';
2222
import { PlanAgentProvider } from '../planAgentProvider';
2323

2424
suite('PlanAgentProvider', () => {
@@ -249,6 +249,24 @@ suite('PlanAgentProvider', () => {
249249
assert.ok(content.includes('vscode/askQuestions'));
250250
});
251251

252+
test('exposes only default read tools plus agent and askQuestions in plan mode by default', async () => {
253+
const provider = createProvider();
254+
const agents = await provider.provideCustomAgents({}, {} as any);
255+
256+
assert.equal(agents.length, 1);
257+
const content = await getAgentContent(agents[0]);
258+
259+
const toolsMatch = content.match(/tools: \[([^\]]+)\]/);
260+
assert.ok(toolsMatch, 'Tools list not found in agent content');
261+
const actualTools = (toolsMatch[1].match(/'([^']+)'/g) || []).map(tool => tool.slice(1, -1)).sort();
262+
const expectedTools = [...DEFAULT_READ_TOOLS, 'agent', 'vscode/askQuestions'].sort();
263+
264+
assert.deepStrictEqual(actualTools, expectedTools);
265+
assert.ok(!actualTools.includes('edit'));
266+
assert.ok(!actualTools.includes('createFile'));
267+
assert.ok(!actualTools.includes('apply_patch'));
268+
});
269+
252270
test('has correct label property', () => {
253271
const provider = createProvider();
254272
assert.ok(provider.label.includes('Plan'));

extensions/copilot/src/extension/intents/node/editCodeIntent.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ export class EditCodeIntent implements IIntent {
123123
const { references } = await renderPromptElement(this.instantiationService, endpoint, ToolCallResultWrapper, { toolCallResults }, undefined, token);
124124
foundReferences.push(...toNewChatReferences(variables, references));
125125
// TODO: how should we splice in the assistant message?
126-
conversation = new Conversation(conversation.sessionId, [...conversation.turns.slice(0, -1), new Turn(latestTurn.id, latestTurn.request, undefined)]);
126+
conversation = new Conversation(conversation.sessionId, [...conversation.turns.slice(0, -1), new Turn(latestTurn.id, latestTurn.request, undefined, [], undefined, undefined, false, latestTurn.modeInstructions)]);
127127
}
128128
return { conversation, request: { ...request, references: [...request.references, ...foundReferences], toolReferences: request.toolReferences.filter((r) => r.name !== CodebaseTool.toolName) } };
129129
}

extensions/copilot/src/extension/intents/node/toolCallingLoop.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,10 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
192192
private readonly _onDidReceiveResponse = this._register(new Emitter<IToolCallingResponseEvent>());
193193
public readonly onDidReceiveResponse = this._onDidReceiveResponse.event;
194194

195+
protected get currentToolCallRounds(): readonly IToolCallRound[] {
196+
return this.toolCallRounds;
197+
}
198+
195199
private get turn() {
196200
return this.options.conversation.getLatestTurn();
197201
}

extensions/copilot/src/extension/prompt/common/conversation.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export class Turn {
8080
request.editedFileEvents,
8181
request.acceptedConfirmationData,
8282
isToolCallLimitAcceptance(request) || isContinueOnError(request) || isSwitchToAutoOnRateLimit(request),
83+
request.modeInstructions2,
8384
);
8485
}
8586

@@ -90,7 +91,8 @@ export class Turn {
9091
private readonly _toolReferences: readonly InternalToolReference[] = [],
9192
readonly editedFileEvents?: ChatRequestEditedFileEvent[],
9293
readonly acceptedConfirmationData?: unknown[],
93-
readonly isContinuation = false
94+
readonly isContinuation = false,
95+
readonly modeInstructions?: ChatRequest['modeInstructions2'],
9496
) { }
9597

9698
get promptVariables(): ChatVariablesCollection | undefined {

extensions/copilot/src/extension/prompt/node/chatParticipantRequestHandler.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,10 @@ function createTurnFromVSCodeChatHistoryTurns(
395395
{ message: chatRequestTurn.prompt, type: 'user' },
396396
new ChatVariablesCollection(chatRequestTurn.references),
397397
chatRequestTurn.toolReferences.map(InternalToolReference.from),
398-
chatRequestAsTurn2.editedFileEvents
398+
chatRequestAsTurn2.editedFileEvents,
399+
undefined,
400+
false,
401+
chatRequestAsTurn2.modeInstructions2,
399402
);
400403

401404
// Take just the content messages

extensions/copilot/src/extension/prompt/node/defaultIntentRequestHandler.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,8 +695,10 @@ class DefaultToolCallingLoop extends ToolCallingLoop<IDefaultToolLoopOptions> {
695695
const rawEffort = this.options.request.modelConfiguration?.reasoningEffort;
696696
const reasoningEffort = typeof rawEffort === 'string' ? rawEffort : undefined;
697697
const isSubagent = !!this.options.request.subAgentInvocationId;
698+
const modeChanged = this.didModeChangeSincePreviousRequest();
698699
return this.options.invocation.endpoint.makeChatRequest2({
699700
...opts,
701+
modeChanged,
700702
modelCapabilities: {
701703
...opts.modelCapabilities,
702704
enableThinking: isThinkingLocation && opts.modelCapabilities?.enableThinking,
@@ -737,6 +739,36 @@ class DefaultToolCallingLoop extends ToolCallingLoop<IDefaultToolLoopOptions> {
737739
}, token);
738740
}
739741

742+
private didModeChangeSincePreviousRequest(): boolean {
743+
if (this.options.invocation.endpoint.apiType !== 'responses') {
744+
return false;
745+
}
746+
747+
// Once a mode-switched turn has successfully produced a fresh responses-api
748+
// stateful marker, later requests in the same turn should resume from that
749+
// new chain instead of continuing to invalidate previous_response_id.
750+
// This is especially important for websocket follow-up requests after tool
751+
// calls: keeping modeChanged=true for the entire turn would force the full
752+
// pre-switch history back into every follow-up request, which can pull the
753+
// model back toward the prior mode (for example implementation after
754+
// switching into Plan mode).
755+
if (this.currentToolCallRounds.some(round => !!round.statefulMarker)) {
756+
return false;
757+
}
758+
759+
const previousModeInstructions = this.options.conversation.turns.at(-2)?.modeInstructions;
760+
if (!previousModeInstructions && !this.options.request.modeInstructions2) {
761+
return false;
762+
}
763+
764+
const modeChanged = !areModeInstructionsEqual(previousModeInstructions, this.options.request.modeInstructions2);
765+
if (modeChanged) {
766+
this._logService.trace('[DefaultIntentRequestHandler] Detected mode instructions changed between requests');
767+
}
768+
769+
return modeChanged;
770+
}
771+
740772
protected override async getAvailableTools(outputStream: ChatResponseStream | undefined, token: CancellationToken): Promise<LanguageModelToolInformation[]> {
741773
const tools = await this.options.invocation.getAvailableTools?.() ?? [];
742774

@@ -793,3 +825,39 @@ class DefaultToolCallingLoop extends ToolCallingLoop<IDefaultToolLoopOptions> {
793825
interface IInternalRequestResult extends IToolCallLoopResult {
794826
lastRequestTelemetry: ChatTelemetry;
795827
}
828+
829+
type ModeInstructions = NonNullable<ChatRequest['modeInstructions2']>;
830+
type ModeInstructionMetadata = ModeInstructions['metadata'];
831+
832+
function areModeInstructionsEqual(a: ChatRequest['modeInstructions2'], b: ChatRequest['modeInstructions2']): boolean {
833+
if (!a || !b) {
834+
return a === b;
835+
}
836+
837+
return a.uri?.toString() === b.uri?.toString()
838+
&& a.name === b.name
839+
&& a.content === b.content
840+
&& a.isBuiltin === b.isBuiltin
841+
&& serializeModeInstructionMetadata(a.metadata) === serializeModeInstructionMetadata(b.metadata);
842+
}
843+
844+
function normalizeModeInstructionMetadata(metadata: ModeInstructionMetadata): Record<string, boolean | string | number> | undefined {
845+
if (!metadata) {
846+
return undefined;
847+
}
848+
849+
const entries = Object.entries(metadata).sort(([left], [right]) => left.localeCompare(right));
850+
if (entries.length === 0) {
851+
return undefined;
852+
}
853+
854+
return entries.reduce<Record<string, boolean | string | number>>((result, [key, value]) => {
855+
result[key] = value;
856+
return result;
857+
}, {});
858+
}
859+
860+
function serializeModeInstructionMetadata(metadata: ModeInstructionMetadata): string | undefined {
861+
const normalizedMetadata = normalizeModeInstructionMetadata(metadata);
862+
return normalizedMetadata ? JSON.stringify(normalizedMetadata) : undefined;
863+
}

extensions/copilot/src/extension/prompt/node/test/defaultIntentRequestHandler.spec.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,46 @@ suite('defaultIntentRequestHandler', () => {
204204
expect(result.metadata?.resolvedModel).toBe('gpt-4o-resolved');
205205
});
206206

207+
test('ignores stateful marker when mode instructions changed on responses api requests', async () => {
208+
const request = new TestChatRequest();
209+
(request as any).modeInstructions2 = { name: 'Agent', content: 'agent instructions', isBuiltin: true };
210+
(endpoint as any).apiType = 'responses';
211+
const requestSpy = vi.spyOn(endpoint, 'makeChatRequest2');
212+
const previousTurn = new Turn(generateUuid(), { message: 'previous', type: 'user' }, undefined, [], undefined, undefined, false, { name: 'Plan', content: 'plan instructions', isBuiltin: true } as any);
213+
const handler = makeHandler({ request, turns: [previousTurn] });
214+
chatResponse[0] = 'some response here :)';
215+
promptResult = {
216+
...nullRenderPromptResult(),
217+
messages: [{ role: Raw.ChatRole.User, content: [toTextPart('hello world!')] }],
218+
};
219+
220+
await handler.getResult();
221+
222+
expect(requestSpy).toHaveBeenCalledOnce();
223+
expect(requestSpy.mock.calls[0][0].modeChanged).toBe(true);
224+
expect(requestSpy.mock.calls[0][0].ignoreStatefulMarker).toBeUndefined();
225+
});
226+
227+
test('preserves default stateful marker behavior when mode instructions are unchanged on responses api requests', async () => {
228+
const request = new TestChatRequest();
229+
(request as any).modeInstructions2 = { name: 'Agent', content: 'agent instructions', isBuiltin: true };
230+
(endpoint as any).apiType = 'responses';
231+
const requestSpy = vi.spyOn(endpoint, 'makeChatRequest2');
232+
const previousTurn = new Turn(generateUuid(), { message: 'previous', type: 'user' }, undefined, [], undefined, undefined, false, { name: 'Agent', content: 'agent instructions', isBuiltin: true } as any);
233+
const handler = makeHandler({ request, turns: [previousTurn] });
234+
chatResponse[0] = 'some response here :)';
235+
promptResult = {
236+
...nullRenderPromptResult(),
237+
messages: [{ role: Raw.ChatRole.User, content: [toTextPart('hello world!')] }],
238+
};
239+
240+
await handler.getResult();
241+
242+
expect(requestSpy).toHaveBeenCalledOnce();
243+
expect(requestSpy.mock.calls[0][0].modeChanged).toBe(false);
244+
expect(requestSpy.mock.calls[0][0].ignoreStatefulMarker).toBeUndefined();
245+
});
246+
207247
test('makes a tool call turn', async () => {
208248
const handler = makeHandler();
209249
chatResponse[0] = [{

extensions/copilot/src/platform/endpoint/node/responsesApi.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export function createResponsesRequestBody(accessor: ServicesAccessor, options:
5757
// undefined if the connection is new or the summary state changed). Never fall
5858
// back to the HTTP marker lookup in that case.
5959
const ignoreStatefulMarker = !!options.ignoreStatefulMarker || !!options.useWebSocket;
60+
const modeChanged = !!options.modeChanged;
6061

6162
// Tool search: when enabled, split tools into non-deferred (included in the request) and deferred
6263
// (excluded from the request entirely). Uses OpenAI's client-executed tool search protocol: we add
@@ -124,7 +125,7 @@ export function createResponsesRequestBody(accessor: ServicesAccessor, options:
124125

125126
const body: IEndpointBody = {
126127
model,
127-
...rawMessagesToResponseAPI(model, options.messages, ignoreStatefulMarker, webSocketStatefulMarker, toolsMap),
128+
...rawMessagesToResponseAPI(model, options.messages, ignoreStatefulMarker, webSocketStatefulMarker, toolsMap, modeChanged),
128129
stream: true,
129130
tools: finalTools.length > 0 ? finalTools : undefined,
130131
// Only a subset of completion post options are supported, and some
@@ -290,7 +291,7 @@ function resolveWebSocketStatefulMarker(accessor: ServicesAccessor, options: ICr
290291
return wsManager.getStatefulMarker(options.conversationId);
291292
}
292293

293-
function rawMessagesToResponseAPI(modelId: string, messages: readonly Raw.ChatMessage[], ignoreStatefulMarker: boolean, webSocketStatefulMarker: string | undefined, toolsMap?: Map<string, OpenAiFunctionTool>): { input: OpenAI.Responses.ResponseInputItem[]; previous_response_id?: string } {
294+
function rawMessagesToResponseAPI(modelId: string, messages: readonly Raw.ChatMessage[], ignoreStatefulMarker: boolean, webSocketStatefulMarker: string | undefined, toolsMap?: Map<string, OpenAiFunctionTool>, modeChanged: boolean = false): { input: OpenAI.Responses.ResponseInputItem[]; previous_response_id?: string } {
294295
const latestCompactionMessageIndex = getLatestCompactionMessageIndex(messages);
295296
const latestCompactionMessage = latestCompactionMessageIndex !== undefined ? createCompactionRoundTripMessage(messages[latestCompactionMessageIndex]) : undefined;
296297

@@ -312,6 +313,11 @@ function rawMessagesToResponseAPI(modelId: string, messages: readonly Raw.ChatMe
312313
}
313314
}
314315

316+
if (modeChanged) {
317+
previousResponseId = undefined;
318+
markerIndex = undefined;
319+
}
320+
315321
if (markerIndex !== undefined) {
316322
// Requests that resume from previous_response_id send only post-marker history,
317323
// but they still need the latest compaction item even when that item predates

0 commit comments

Comments
 (0)