Skip to content

Refactor askUserQuestion method to remove unused stream parameter #3925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DonJayamanne merged 5 commits into from
Feb 24, 2026
Merged

Refactor askUserQuestion method to remove unused stream parameter #3925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
71530dd
Refactor askUserQuestion method to remove unused stream parameter acr…
DonJayamanne Feb 23, 2026
c6cbb7f
Updates
DonJayamanne Feb 23, 2026
10251be
Merge branch 'main' into don/embarrassing-nightingale
DonJayamanne Feb 24, 2026
b4bf5e3
Fix compilation issues after merging main
DonJayamanne Feb 24, 2026
e3199d7
Remove unused type
DonJayamanne Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/extension/agents/copilotcli/node/copilotcliSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
this.logService.warn('[AskQuestionsTool] No stream available, cannot show question carousel');
throw new Error('User skipped question');
}
const answer = await this._userQuestionHandler.askUserQuestion(userInputRequest, this._stream, request.toolInvocationToken, token);
const answer = await this._userQuestionHandler.askUserQuestion(userInputRequest, request.toolInvocationToken, token);
if (!answer) {
throw new Error('User skipped question');
}
Expand Down
4 changes: 2 additions & 2 deletions src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import type { SessionOptions, SweCustomAgent } from '@github/copilot/sdk';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import type { ChatContext, ChatParticipantToolToken, ChatResponseStream } from 'vscode';
import type { ChatContext, ChatParticipantToolToken } from 'vscode';
import { CancellationToken } from 'vscode-languageserver-protocol';
import { IAuthenticationService } from '../../../../../platform/authentication/common/authentication';
import { IConfigurationService } from '../../../../../platform/configuration/common/configurationService';
Expand Down Expand Up @@ -135,7 +135,7 @@ describe('CopilotCLISessionService', () => {
}();
class FakeUserQuestionHandler implements IUserQuestionHandler {
_serviceBrand: undefined;
async askUserQuestion(question: UserInputRequest, stream: ChatResponseStream, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined> {
async askUserQuestion(question: UserInputRequest, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined> {
return undefined;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import type { Session, SessionOptions } from '@github/copilot/sdk';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import type { ChatContext, ChatParticipantToolToken, ChatResponseStream } from 'vscode';
import type { ChatContext, ChatParticipantToolToken } from 'vscode';
import { ILogService } from '../../../../../platform/log/common/logService';
import { NullRequestLogger } from '../../../../../platform/requestLogger/node/nullRequestLogger';
import { IRequestLogger } from '../../../../../platform/requestLogger/node/requestLogger';
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('CopilotCLISession', () => {
async function createSession(): Promise<CopilotCLISession> {
class FakeUserQuestionHandler implements IUserQuestionHandler {
_serviceBrand: undefined;
async askUserQuestion(question: UserInputRequest, stream: ChatResponseStream, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined> {
async askUserQuestion(question: UserInputRequest, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined> {
return undefined;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/extension/agents/copilotcli/node/userInputHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import type { SessionOptions } from '@github/copilot/sdk';
import type { CancellationToken, ChatParticipantToolToken, ChatResponseStream } from 'vscode';
import type { CancellationToken, ChatParticipantToolToken } from 'vscode';
import { createServiceIdentifier } from '../../../../util/common/services';

export type UserInputRequest = Parameters<NonNullable<SessionOptions['requestUserInput']>>[0];
Expand All @@ -15,5 +15,5 @@ export const IUserQuestionHandler = createServiceIdentifier<IUserQuestionHandler

export interface IUserQuestionHandler {
_serviceBrand: undefined;
askUserQuestion(question: UserInputRequest, stream: ChatResponseStream, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined>;
askUserQuestion(question: UserInputRequest, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined>;
}
31 changes: 6 additions & 25 deletions src/extension/chatSessions/vscode-node/askUserQuestionHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { ChatParticipantToolToken, ChatResponseStream, LanguageModelTextPart } from 'vscode';
import { ChatParticipantToolToken, LanguageModelTextPart } from 'vscode';
import { ILogService } from '../../../platform/log/common/logService';
import { CancellationToken } from '../../../util/vs/base/common/cancellation';
import { IUserQuestionHandler, UserInputRequest, UserInputResponse } from '../../agents/copilotcli/node/userInputHelpers';
import { ToolName } from '../../tools/common/toolNames';
import { CopilotToolMode, ICopilotTool } from '../../tools/common/toolsRegistry';
import { IToolsService } from '../../tools/common/toolsService';

export interface IQuestionOption {
Expand Down Expand Up @@ -47,13 +46,7 @@ export class UserQuestionHandler implements IUserQuestionHandler {
@IToolsService private readonly _toolsService: IToolsService,
) {
}
async askUserQuestion(question: UserInputRequest, stream: ChatResponseStream, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined> {
// Get the AskQuestions tool instance directly
const askQuestionsTool = this._toolsService.getCopilotTool(ToolName.CoreAskQuestions) as ICopilotTool<IAskQuestionsParams> | undefined;
if (!askQuestionsTool?.invoke) {
throw new Error('AskQuestions tool is not available');
}

async askUserQuestion(question: UserInputRequest, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined> {
const input: IAskQuestionsParams = {
questions: [
{
Expand All @@ -64,23 +57,11 @@ export class UserQuestionHandler implements IUserQuestionHandler {
}
]
};
// Call resolveInput to inject the stream (needed for displaying the question carousel)
if (askQuestionsTool.resolveInput) {
await askQuestionsTool.resolveInput(
input,
{ stream } as Parameters<typeof askQuestionsTool.resolveInput>[1],
CopilotToolMode.FullContext
);
}
const result = await this._toolsService.invokeTool(ToolName.CoreAskQuestions, {
input,
toolInvocationToken,
}, token);

// Invoke the tool directly
const result = await askQuestionsTool.invoke(
{
input: input,
toolInvocationToken,
},
token
);

// Parse the result
const firstPart = result?.content.at(0);
Expand Down
50 changes: 12 additions & 38 deletions src/extension/chatSessions/vscode-node/test/askUserQuestionHandler.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
*--------------------------------------------------------------------------------------------*/

import { describe, expect, it, vi } from 'vitest';
import { LanguageModelToolInvocationOptions } from 'vscode';
import { mock } from '../../../../util/common/test/simpleMock';
import { CancellationToken } from '../../../../util/vs/base/common/cancellation';
import { LanguageModelTextPart, LanguageModelToolResult } from '../../../../vscodeTypes';
import { LanguageModelTextPart, LanguageModelToolResult, LanguageModelToolResult2 } from '../../../../vscodeTypes';
import { UserInputRequest } from '../../../agents/copilotcli/node/userInputHelpers';
import { CopilotToolMode, ICopilotTool } from '../../../tools/common/toolsRegistry';
import { ICopilotTool } from '../../../tools/common/toolsRegistry';
import { IToolsService } from '../../../tools/common/toolsService';
import { IAnswerResult, UserQuestionHandler } from '../askUserQuestionHandler';

Expand All @@ -21,8 +22,8 @@ function makeAskQuestionsTool(invokeResult: LanguageModelToolResult | undefined,

function makeToolsService(tool: ICopilotTool<unknown> | undefined): IToolsService {
return new class extends mock<IToolsService>() {
override getCopilotTool(name: string) {
return tool as unknown as ReturnType<IToolsService['getCopilotTool']>;
override invokeTool(name: string, options: LanguageModelToolInvocationOptions<unknown>, token: CancellationToken): Thenable<LanguageModelToolResult2> {
return (tool as any).invoke(options, token) as Thenable<LanguageModelToolResult2>;
}
}();
}
Expand All @@ -36,7 +37,6 @@ function makeHandler(tool: ICopilotTool<unknown> | undefined) {
return new UserQuestionHandler(logService, makeToolsService(tool));
}

const stream = {} as import('vscode').ChatResponseStream;
const toolInvocationToken = {} as import('vscode').ChatParticipantToolToken;

const question: UserInputRequest = {
Expand All @@ -47,31 +47,25 @@ const question: UserInputRequest = {

describe('UserQuestionHandler', () => {
describe('askUserQuestion', () => {
it('throws when AskQuestions tool is unavailable', async () => {
const handler = makeHandler(undefined);
await expect(handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None))
.rejects.toThrow('AskQuestions tool is not available');
});

it('returns undefined when tool returns no content', async () => {
const tool = makeAskQuestionsTool(new LanguageModelToolResult([]));
const handler = makeHandler(tool);
const result = await handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None);
const result = await handler.askUserQuestion(question, toolInvocationToken, CancellationToken.None);
expect(result).toBeUndefined();
});

it('returns undefined when result part is not a LanguageModelTextPart', async () => {
const tool = makeAskQuestionsTool(new LanguageModelToolResult([{ value: 'not-a-text-part' } as unknown as LanguageModelTextPart]));
const handler = makeHandler(tool);
const result = await handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None);
const result = await handler.askUserQuestion(question, toolInvocationToken, CancellationToken.None);
expect(result).toBeUndefined();
});

it('returns undefined when the answer key is missing', async () => {
const answers: IAnswerResult = { answers: {} };
const tool = makeAskQuestionsTool(new LanguageModelToolResult([new LanguageModelTextPart(JSON.stringify(answers))]));
const handler = makeHandler(tool);
const result = await handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None);
const result = await handler.askUserQuestion(question, toolInvocationToken, CancellationToken.None);
expect(result).toBeUndefined();
});

Expand All @@ -83,7 +77,7 @@ describe('UserQuestionHandler', () => {
};
const tool = makeAskQuestionsTool(new LanguageModelToolResult([new LanguageModelTextPart(JSON.stringify(answers))]));
const handler = makeHandler(tool);
const result = await handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None);
const result = await handler.askUserQuestion(question, toolInvocationToken, CancellationToken.None);
expect(result).toEqual({ answer: 'Purple', wasFreeform: true });
});

Expand All @@ -95,7 +89,7 @@ describe('UserQuestionHandler', () => {
};
const tool = makeAskQuestionsTool(new LanguageModelToolResult([new LanguageModelTextPart(JSON.stringify(answers))]));
const handler = makeHandler(tool);
const result = await handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None);
const result = await handler.askUserQuestion(question, toolInvocationToken, CancellationToken.None);
expect(result).toEqual({ answer: 'Red, Blue', wasFreeform: false });
});

Expand All @@ -107,37 +101,17 @@ describe('UserQuestionHandler', () => {
};
const tool = makeAskQuestionsTool(new LanguageModelToolResult([new LanguageModelTextPart(JSON.stringify(answers))]));
const handler = makeHandler(tool);
const result = await handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None);
const result = await handler.askUserQuestion(question, toolInvocationToken, CancellationToken.None);
expect(result).toBeUndefined();
});

it('calls resolveInput when available to inject the stream', async () => {
const resolveInput = vi.fn(async () => { });
const answers: IAnswerResult = {
answers: {
'What color?': { selected: ['Red'], freeText: null, skipped: false }
}
};
const tool = makeAskQuestionsTool(
new LanguageModelToolResult([new LanguageModelTextPart(JSON.stringify(answers))]),
resolveInput as unknown as ICopilotTool<unknown>['resolveInput']
);
const handler = makeHandler(tool);
await handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None);
expect(resolveInput).toHaveBeenCalledWith(
expect.objectContaining({ questions: expect.any(Array) }),
expect.objectContaining({ stream }),
CopilotToolMode.FullContext
);
});

it('passes question text, choices, and freeform flag to the tool', async () => {
const answers: IAnswerResult = {
answers: { 'What color?': { selected: ['Red'], freeText: null, skipped: false } }
};
const tool = makeAskQuestionsTool(new LanguageModelToolResult([new LanguageModelTextPart(JSON.stringify(answers))]));
const handler = makeHandler(tool);
await handler.askUserQuestion(question, stream, toolInvocationToken, CancellationToken.None);
await handler.askUserQuestion(question, toolInvocationToken, CancellationToken.None);
const invokeArg = (tool.invoke as ReturnType<typeof vi.fn>).mock.calls[0][0];
expect(invokeArg.input.questions[0]).toMatchObject({
header: 'What color?',
Expand Down
2 changes: 1 addition & 1 deletion src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => {
const fileSystem = new MockFileSystemService();
class FakeUserQuestionHandler implements IUserQuestionHandler {
_serviceBrand: undefined;
async askUserQuestion(question: UserInputRequest, stream: vscode.ChatResponseStream, toolInvocationToken: vscode.ChatParticipantToolToken, token: vscode.CancellationToken): Promise<UserInputResponse | undefined> {
async askUserQuestion(question: UserInputRequest, toolInvocationToken: vscode.ChatParticipantToolToken, token: vscode.CancellationToken): Promise<UserInputResponse | undefined> {
return undefined;
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/cli.stest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as fs from 'fs/promises';
import * as http from 'http';
import { platform, tmpdir } from 'os';
import * as path from 'path';
import type { ChatParticipantToolToken, ChatPromptReference, ChatResponseStream } from 'vscode';
import type { ChatParticipantToolToken, ChatPromptReference } from 'vscode';
import { ICustomSessionTitleService } from '../../src/extension/agents/copilotcli/common/customSessionTitleService';
import { ChatDelegationSummaryService, IChatDelegationSummaryService } from '../../src/extension/agents/copilotcli/common/delegationSummaryService';
import { CopilotCLIAgents, CopilotCLIModels, CopilotCLISDK, CopilotCLISessionOptions, ICopilotCLIAgents, ICopilotCLIModels, ICopilotCLISDK } from '../../src/extension/agents/copilotcli/node/copilotCli';
Expand Down Expand Up @@ -196,7 +196,7 @@ async function registerChatServices(testingServiceCollection: TestingServiceColl
constructor(
) {
}
async askUserQuestion(question: UserInputRequest, stream: ChatResponseStream, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined> {
async askUserQuestion(question: UserInputRequest, toolInvocationToken: ChatParticipantToolToken, token: CancellationToken): Promise<UserInputResponse | undefined> {
return undefined;
}
}
Expand Down