Skip to content

restore history sharing rules for agents #233930

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
jrieken merged 1 commit into from
Nov 15, 2024
Merged

restore history sharing rules for agents #233930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ suite('chat', () => {
assert.deepStrictEqual(result.metadata, { key: 'value' });
});

test('participant history not isolated within extension', async () => {
test('isolated participant history', async () => {
const onRequest = setupParticipant();
const onRequest2 = setupParticipant(true);

Expand All @@ -132,13 +132,13 @@ suite('chat', () => {
commands.executeCommand('workbench.action.chat.open', { query: '@participant2 hi' });
}, 0);
const request2 = await asPromise(onRequest2);
assert.strictEqual(request2.context.history.length, 10);
assert.strictEqual(request2.context.history.length, 0);

setTimeout(() => {
commands.executeCommand('workbench.action.chat.open', { query: '@participant2 hi' });
}, 0);
const request3 = await asPromise(onRequest2);
assert.strictEqual(request3.context.history.length, 12); // request + response = 2
assert.strictEqual(request3.context.history.length, 2); // request + response = 2
});

test('title provider is called for first request', async () => {
Expand Down
11 changes: 3 additions & 8 deletions src/vs/workbench/api/common/extHostChatAgents2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,10 +515,9 @@ export class ExtHostChatAgents2 extends Disposable implements ExtHostChatAgentsS

for (const h of context.history) {
const ehResult = typeConvert.ChatAgentResult.to(h.result);
// get full result when agent is the same or agent is from the same extension
const result: vscode.ChatResult = agentId === h.request.agentId || ExtensionIdentifier.equals(this.getChatAgentById(agentId)?.extension.identifier, this.getChatAgentById(h.request.agentId)?.extension.identifier)
? ehResult
: { ...ehResult, metadata: undefined };
const result: vscode.ChatResult = agentId === h.request.agentId ?
ehResult :
{ ...ehResult, metadata: undefined };

// REQUEST turn
const varsWithoutTools = h.request.variables.variables
Expand All @@ -538,10 +537,6 @@ export class ExtHostChatAgents2 extends Disposable implements ExtHostChatAgentsS
return res;
}

private getChatAgentById(id: string) {
return Iterable.find(this._agents.values(), a => a.id === id);
}

$releaseSession(sessionId: string): void {
this._sessionDisposables.deleteAndDispose(sessionId);
}
Expand Down
15 changes: 2 additions & 13 deletions src/vs/workbench/contrib/chat/common/chatServiceImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { StopWatch } from '../../../../base/common/stopwatch.js';
import { URI, UriComponents } from '../../../../base/common/uri.js';
import { localize } from '../../../../nls.js';
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
import { ExtensionIdentifier } from '../../../../platform/extensions/common/extensions.js';
import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
import { ILogService } from '../../../../platform/log/common/log.js';
import { Progress } from '../../../../platform/progress/common/progress.js';
Expand Down Expand Up @@ -785,23 +784,13 @@ export class ChatService extends Disposable implements IChatService {

private getHistoryEntriesFromModel(requests: IChatRequestModel[], sessionId: string, location: ChatAgentLocation, forAgentId: string): IChatAgentHistoryEntry[] {
const history: IChatAgentHistoryEntry[] = [];


const defaultAgentId = this.chatAgentService.getDefaultAgent(location)?.id;

const forAgentExtension = this.chatAgentService.getAgent(forAgentId);

for (const request of requests) {
if (!request.response) {
continue;
}

const responseAgentExtension = request.response.agent ? this.chatAgentService.getAgent(request.response.agent.id) : undefined;

if (forAgentId !== defaultAgentId
&& forAgentId !== request.response.agent?.id
&& !ExtensionIdentifier.equals(forAgentExtension?.extensionId, responseAgentExtension?.extensionId)
) {
const defaultAgentId = this.chatAgentService.getDefaultAgent(location)?.id;
if (forAgentId !== request.response.agent?.id && forAgentId !== defaultAgentId) {
// An agent only gets to see requests that were sent to this agent.
// The default agent (the undefined case) gets to see all of them.
continue;
Expand Down
18 changes: 4 additions & 14 deletions src/vs/workbench/contrib/chat/test/common/chatService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import { IExtensionService, nullExtensionDescription } from '../../../../service
import { IViewsService } from '../../../../services/views/common/viewsService.js';
import { TestContextService, TestExtensionService, TestStorageService } from '../../../../test/common/workbenchTestServices.js';
import { MarkdownString } from '../../../../../base/common/htmlContent.js';
import { ExtensionIdentifier } from '../../../../../platform/extensions/common/extensions.js';

const chatAgentWithUsedContextId = 'ChatProviderWithUsedContext';
const chatAgentWithUsedContext: IChatAgent = {
Expand Down Expand Up @@ -197,11 +196,9 @@ suite('ChatService', () => {
};

testDisposables.add(chatAgentService.registerAgent('defaultAgent', { ...getAgentData('defaultAgent'), isDefault: true }));
testDisposables.add(chatAgentService.registerAgent('agent2', { ...getAgentData('agent2'), extensionId: new ExtensionIdentifier('foo.bar') }));
testDisposables.add(chatAgentService.registerAgent('agent3', getAgentData('agent3')));
testDisposables.add(chatAgentService.registerAgent('agent2', getAgentData('agent2')));
testDisposables.add(chatAgentService.registerAgentImplementation('defaultAgent', historyLengthAgent));
testDisposables.add(chatAgentService.registerAgentImplementation('agent2', historyLengthAgent));
testDisposables.add(chatAgentService.registerAgentImplementation('agent3', historyLengthAgent));

const testService = testDisposables.add(instantiationService.createInstance(ChatService));
const model = testDisposables.add(testService.startSession(ChatAgentLocation.Panel, CancellationToken.None));
Expand All @@ -220,19 +217,12 @@ suite('ChatService', () => {
assert.strictEqual(model.getRequests().length, 2);
assert.strictEqual(model.getRequests()[1].response?.result?.metadata?.historyLength, 0);

// Send a request to agent3- it can see the default agent's message because they are from the same extension
const response3 = await testService.sendRequest(model.sessionId, `test request`, { agentId: 'agent3' });
// Send a request to defaultAgent - the default agent can see agent2's message
const response3 = await testService.sendRequest(model.sessionId, `test request`, { agentId: 'defaultAgent' });
assert(response3);
await response3.responseCompletePromise;
assert.strictEqual(model.getRequests().length, 3);
assert.strictEqual(model.getRequests()[2].response?.result?.metadata?.historyLength, 1);

// Send a request to defaultAgent - the default agent can see agent2's message
const response4 = await testService.sendRequest(model.sessionId, `test request`, { agentId: 'defaultAgent' });
assert(response4);
await response4.responseCompletePromise;
assert.strictEqual(model.getRequests().length, 4);
assert.strictEqual(model.getRequests()[3].response?.result?.metadata?.historyLength, 3);
assert.strictEqual(model.getRequests()[2].response?.result?.metadata?.historyLength, 2);
});

test('can serialize', async () => {
Expand Down