Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 2 deletions src/vs/platform/agentHost/common/sessionConfigKeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ export const enum SessionConfigKey {
Isolation = 'isolation',
/** `'branch'` — base branch to work from. */
Branch = 'branch',
/** `'branchNameHint'` — client-supplied hint used during worktree creation. */
BranchNameHint = 'branchNameHint',
/** `'mode'` — agent execution mode (interactive / plan). */
Mode = 'mode',
}
Expand Down
37 changes: 30 additions & 7 deletions src/vs/platform/agentHost/node/copilot/copilotAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,28 @@ export function getCopilotWorktreeBranchName(sessionId: string, branchNameHint:
return `agents/${branchNameHint ? `${branchNameHint}-${sessionId.substring(0, 8)}` : sessionId}`;
}

/**
* Derive a slug-style branch-name hint from the user's first message. Used
* by the worktree isolation flow so the generated branch name reflects the
* intent of the session instead of being just a session id.
*
* Returns `undefined` if the message has no slug-able content (e.g. only
* punctuation), in which case the caller falls back to a session-id-only
* branch name.
*/
export function getCopilotBranchNameHintFromMessage(message: string): string | undefined {
const words = message
.toLowerCase()
.normalize('NFKD')
.replace(/[^a-z0-9]+/g, '-')
.replace(/^-+|-+$/g, '')
.split('-')
.filter(word => word.length > 0)
.slice(0, 8);
const hint = words.join('-').slice(0, 48).replace(/-+$/g, '');
return hint.length > 0 ? hint : undefined;
}
Comment thread
roblourens marked this conversation as resolved.

/**
* Builds the localized "Created isolated worktree for branch X" markdown
* shown at the top of the first response in worktree-isolated sessions.
Expand Down Expand Up @@ -761,7 +783,7 @@ export class CopilotAgent extends Disposable implements IAgent {
* `SessionConfigChanged` actions that arrived after `createSession` are
* honoured without bespoke forwarding.
*/
private async _materializeProvisional(sessionId: string): Promise<CopilotAgentSession> {
private async _materializeProvisional(sessionId: string, prompt: string): Promise<CopilotAgentSession> {
const provisional = this._provisionalSessions.get(sessionId);
if (!provisional) {
throw new Error(`Cannot materialize unknown provisional session: ${sessionId}`);
Expand All @@ -781,7 +803,7 @@ export class CopilotAgent extends Disposable implements IAgent {
const customizationDirectory = provisional.workingDirectory;
const activeClient = this._activeClients.get(sessionUri);
const snapshot = activeClient ? await activeClient.snapshot(customizationDirectory) : undefined;
const workingDirectory = await this._resolveSessionWorkingDirectory(materializedConfig, sessionId);
const workingDirectory = await this._resolveSessionWorkingDirectory(materializedConfig, sessionId, prompt);
const shellManager = this._instantiationService.createInstance(ShellManager, sessionUri, workingDirectory);
const sessionConfigBuilder = this._buildSessionConfig(snapshot, shellManager);

Expand Down Expand Up @@ -920,10 +942,12 @@ export class CopilotAgent extends Disposable implements IAgent {
await this._sessionSequencer.queue(sessionId, async () => {

// First message on a provisional session: materialize the SDK
// session, worktree, and on-disk metadata before continuing.
// session, worktree, and on-disk metadata before continuing. The
// prompt is forwarded so a worktree-isolated session can derive
// its branch-name hint from the user's first message.
let entry: CopilotAgentSession | undefined;
if (this._provisionalSessions.has(sessionId)) {
entry = await this._materializeProvisional(sessionId);
entry = await this._materializeProvisional(sessionId, prompt);
} else {
entry = this._sessions.get(sessionId);
}
Expand Down Expand Up @@ -1430,7 +1454,7 @@ export class CopilotAgent extends Disposable implements IAgent {
return this._gitService.getBranches(workingDirectory, { query, limit: CopilotAgent._BRANCH_COMPLETION_LIMIT });
}

protected async _resolveSessionWorkingDirectory(config: IAgentCreateSessionConfig | undefined, sessionId: string): Promise<URI | undefined> {
protected async _resolveSessionWorkingDirectory(config: IAgentCreateSessionConfig | undefined, sessionId: string, prompt?: string): Promise<URI | undefined> {
if (config?.config?.isolation !== 'worktree' || !config.workingDirectory || typeof config.config.branch !== 'string') {
return config?.workingDirectory;
}
Expand All @@ -1441,8 +1465,7 @@ export class CopilotAgent extends Disposable implements IAgent {
}

const worktreesRoot = getCopilotWorktreesRoot(repositoryRoot);
const branchNameHintRaw = config.config[SessionConfigKey.BranchNameHint];
const branchNameHint = typeof branchNameHintRaw === 'string' ? branchNameHintRaw : undefined;
const branchNameHint = prompt ? getCopilotBranchNameHintFromMessage(prompt) : undefined;
const branchName = getCopilotWorktreeBranchName(sessionId, branchNameHint);
const worktree = URI.joinPath(worktreesRoot, getCopilotWorktreeName(branchName));
await fs.mkdir(worktreesRoot.fsPath, { recursive: true });
Expand Down
28 changes: 19 additions & 9 deletions src/vs/platform/agentHost/test/node/copilotAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { AgentConfigurationService, IAgentConfigurationService } from '../../nod
import { AgentHostStateManager } from '../../node/agentHostStateManager.js';
import { IAgentHostGitService } from '../../node/agentHostGitService.js';
import { IAgentHostTerminalManager } from '../../node/agentHostTerminalManager.js';
import { CopilotAgent, getCopilotWorktreeBranchName, getCopilotWorktreeName, getCopilotWorktreesRoot } from '../../node/copilot/copilotAgent.js';
import { CopilotAgent, getCopilotBranchNameHintFromMessage, getCopilotWorktreeBranchName, getCopilotWorktreeName, getCopilotWorktreesRoot } from '../../node/copilot/copilotAgent.js';
import { CopilotAgentSession, type SessionWrapperFactory } from '../../node/copilot/copilotAgentSession.js';
import { CopilotSessionWrapper } from '../../node/copilot/copilotSessionWrapper.js';
import { ShellManager } from '../../node/copilot/copilotShellTools.js';
Expand Down Expand Up @@ -273,8 +273,8 @@ class TestableCopilotAgent extends CopilotAgent {
return stub;
}

resolveWorktreeForTest(config: Parameters<CopilotAgent['createSession']>[0], sessionId: string): Promise<URI | undefined> {
return this._resolveSessionWorkingDirectory(config, sessionId);
resolveWorktreeForTest(config: Parameters<CopilotAgent['createSession']>[0], sessionId: string, prompt?: string): Promise<URI | undefined> {
return this._resolveSessionWorkingDirectory(config, sessionId, prompt);
}
}

Expand Down Expand Up @@ -372,6 +372,16 @@ suite('CopilotAgent', () => {
assert.strictEqual(getCopilotWorktreeBranchName('12345678-aaaa-bbbb-cccc-123456789abc', 'a'.repeat(48)).length, 'agents/'.length + 48 + '-12345678'.length);
});

test('derives slug branch hint from first message', () => {
assert.strictEqual(getCopilotBranchNameHintFromMessage('Add agent host config'), 'add-agent-host-config');
assert.strictEqual(getCopilotBranchNameHintFromMessage(' Fix: the bug!! '), 'fix-the-bug');
assert.strictEqual(getCopilotBranchNameHintFromMessage('Refactor café ☕ rendering'), 'refactor-cafe-rendering');
assert.strictEqual(getCopilotBranchNameHintFromMessage('one two three four five six seven eight nine ten'), 'one-two-three-four-five-six-seven-eight');
assert.strictEqual(getCopilotBranchNameHintFromMessage('a'.repeat(100))?.length, 48);
assert.strictEqual(getCopilotBranchNameHintFromMessage('!!! ??? ...'), undefined);
assert.strictEqual(getCopilotBranchNameHintFromMessage(''), undefined);
});

test('returns empty models and throws AuthRequired for sessions before authentication', async () => {
const agent = createTestAgent(disposables);
try {
Expand Down Expand Up @@ -824,8 +834,8 @@ suite('CopilotAgent', () => {
const expectedBranchName = getCopilotWorktreeBranchName(sessionId, branchHint);
const workingDir = await agent.resolveWorktreeForTest({
workingDirectory: repositoryRoot,
config: { isolation: 'worktree', branch: 'main', branchNameHint: branchHint },
}, sessionId);
config: { isolation: 'worktree', branch: 'main' },
}, sessionId, 'Add feature');
assert.ok(workingDir, 'resolveWorktreeForTest must return a worktree URI');
assert.deepStrictEqual(gitService.addedWorktrees.length, 1, 'addWorktree must be called once');
assert.strictEqual(gitService.addedWorktrees[0].branchName, expectedBranchName);
Expand Down Expand Up @@ -950,7 +960,7 @@ suite('CopilotAgent', () => {
await agent.authenticate('https://api.github.com', 'token');
const workingDir = await agent.resolveWorktreeForTest({
workingDirectory: repositoryRoot,
config: { isolation: 'worktree', branch: 'main', branchNameHint: 'feat' },
config: { isolation: 'worktree', branch: 'main' },
}, sessionId);
assert.ok(workingDir, 'worktree must be created');
// Simulate the worktree directory existing on disk so the archive
Expand Down Expand Up @@ -997,7 +1007,7 @@ suite('CopilotAgent', () => {
await agent.authenticate('https://api.github.com', 'token');
const workingDir = await agent.resolveWorktreeForTest({
workingDirectory: repositoryRoot,
config: { isolation: 'worktree', branch: 'main', branchNameHint: 'feat' },
config: { isolation: 'worktree', branch: 'main' },
}, sessionId);
await fs.mkdir(workingDir!.fsPath, { recursive: true });
gitService.dirtyWorkingDirectories.add(workingDir!.fsPath);
Expand Down Expand Up @@ -1028,7 +1038,7 @@ suite('CopilotAgent', () => {
await agent.authenticate('https://api.github.com', 'token');
const workingDir = await agent.resolveWorktreeForTest({
workingDirectory: repositoryRoot,
config: { isolation: 'worktree', branch: 'main', branchNameHint: 'feat' },
config: { isolation: 'worktree', branch: 'main' },
}, sessionId);
await fs.mkdir(workingDir!.fsPath, { recursive: true });
// Drop the branch so cleanup must skip.
Expand Down Expand Up @@ -1083,7 +1093,7 @@ suite('CopilotAgent', () => {
await agent.authenticate('https://api.github.com', 'token');
const workingDir = await agent.resolveWorktreeForTest({
workingDirectory: repositoryRoot,
config: { isolation: 'worktree', branch: 'main', branchNameHint: 'feat' },
config: { isolation: 'worktree', branch: 'main' },
}, sessionId);
await fs.mkdir(workingDir!.fsPath, { recursive: true });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,6 @@ export class AgentHostSessionConfigPicker extends Disposable {
const properties = this._orderProperties(Object.entries(resolvedConfig.schema.properties));

for (const [property, schema] of properties) {
if (property === SessionConfigKey.BranchNameHint) {
continue;
}
// Only render pickers for properties we know how to present. Today
// that's string properties with either a static `enum` or a
// dynamic enum sourced via `getSessionConfigCompletions`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { AgentHostSessionListController } from './agentHostSessionListController
import { LoggingAgentConnection } from './loggingAgentConnection.js';
import { SyncedCustomizationBundler } from './syncedCustomizationBundler.js';

export { AgentHostSessionHandler, getAgentHostBranchNameHint } from './agentHostSessionHandler.js';
export { AgentHostSessionHandler } from './agentHostSessionHandler.js';
export { AgentHostSessionListController } from './agentHostSessionListController.js';

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { isLocation, type Location } from '../../../../../../editor/common/langu
import { IPosition } from '../../../../../../editor/common/core/position.js';
import { localize } from '../../../../../../nls.js';
import { AgentProvider, AgentSession, type IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js';
import { SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js';
import { IAgentSubscription, observableFromSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js';
import { SessionTruncatedAction } from '../../../../../../platform/agentHost/common/state/protocol/actions.js';
import { CompletionItemKind as AhpCompletionItemKind, type CompletionItem as AhpCompletionItem } from '../../../../../../platform/agentHost/common/state/protocol/commands.js';
Expand Down Expand Up @@ -336,19 +335,6 @@ export interface IAgentHostSessionHandlerConfig {
readonly customizations?: IObservable<CustomizationRef[]>;
}

export function getAgentHostBranchNameHint(message: string): string | undefined {
const words = message
.toLowerCase()
.normalize('NFKD')
.replace(/[^a-z0-9]+/g, '-')
.replace(/^-+|-+$/g, '')
.split('-')
.filter(word => word.length > 0)
.slice(0, 8);
const hint = words.join('-').slice(0, 48).replace(/-+$/g, '');
return hint.length > 0 ? hint : undefined;
}

/**
* Converts a UTF-16 code-unit offset in `text` to a 1-based Monaco
* `IPosition`. Used to translate AHP completion-item ranges (which use
Expand All @@ -368,7 +354,6 @@ function offsetToPosition(text: string, offset: number): IPosition {
}
return { lineNumber, column };
}

export class AgentHostSessionHandler extends Disposable implements IChatSessionContentProvider {

private readonly _activeSessions = new ResourceMap<AgentHostChatSession>();
Expand Down Expand Up @@ -744,7 +729,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC
// folder-pick time, or this session was created via a legacy/
// test path). Fall back to the original create-then-subscribe
// flow.
await this._createAndSubscribe(request.sessionResource, this._createModelSelection(request.userSelectedModelId, request.modelConfiguration), undefined, request.agentHostSessionConfig, getAgentHostBranchNameHint(request.message));
await this._createAndSubscribe(request.sessionResource, this._createModelSelection(request.userSelectedModelId, request.modelConfiguration), undefined, request.agentHostSessionConfig);
} else {
// Eager-created session: take a refcounted subscription so the
// handler observes state changes for the duration of the chat
Expand Down Expand Up @@ -2285,8 +2270,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC
}

/** Creates a new backend session and subscribes to its state. */
private async _createAndSubscribe(sessionResource: URI, model: ModelSelection | undefined, fork?: { session: URI; turnIndex: number; turnId: string }, sessionConfig?: Record<string, unknown>, branchNameHint?: string): Promise<URI> {
const config = branchNameHint ? { ...sessionConfig, [SessionConfigKey.BranchNameHint]: branchNameHint } : sessionConfig;
private async _createAndSubscribe(sessionResource: URI, model: ModelSelection | undefined, fork?: { session: URI; turnIndex: number; turnId: string }, config?: Record<string, unknown>): Promise<URI> {
const workingDirectory = this._resolveRequestedWorkingDirectory(sessionResource);
const requestedSession = fork ? undefined : this._resolveSessionUri(sessionResource);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { IProductService } from '../../../../../../platform/product/common/produ
import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
import { IOutputService } from '../../../../../services/output/common/output.js';
import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js';
import { AgentHostContribution, AgentHostSessionListController, AgentHostSessionHandler, getAgentHostBranchNameHint } from '../../../browser/agentSessions/agentHost/agentHostChatContribution.js';
import { AgentHostContribution, AgentHostSessionListController, AgentHostSessionHandler } from '../../../browser/agentSessions/agentHost/agentHostChatContribution.js';
import { AgentHostLanguageModelProvider } from '../../../browser/agentSessions/agentHost/agentHostLanguageModelProvider.js';
import { IFileService } from '../../../../../../platform/files/common/files.js';
import { TestFileService } from '../../../../../test/common/workbenchTestServices.js';
Expand All @@ -53,7 +53,6 @@ import { IAgentHostTerminalService } from '../../../../terminal/browser/agentHos
import { IAgentHostSessionWorkingDirectoryResolver } from '../../../browser/agentSessions/agentHost/agentHostSessionWorkingDirectoryResolver.js';
import { ILanguageModelToolsService } from '../../../common/tools/languageModelToolsService.js';
import { IPromptsService } from '../../../common/promptSyntax/service/promptsService.js';
import { SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js';
import { IChatWidgetService } from '../../../browser/chat.js';
import { ChatQuestionCarouselData } from '../../../common/model/chatProgressTypes/chatQuestionCarouselData.js';

Expand Down Expand Up @@ -2659,20 +2658,46 @@ suite('AgentHostChatContribution', () => {
await turnPromise;

assert.strictEqual(agentHostService.createSessionCalls.length, 1);
assert.deepStrictEqual(agentHostService.createSessionCalls[0].config, { ...config, [SessionConfigKey.BranchNameHint]: 'add-agent-host-session-configuration-flow' });
}));

test('handler derives deterministic branch name hints from first request text', () => {
assert.deepStrictEqual([
getAgentHostBranchNameHint('Add Agent Host session configuration flow'),
getAgentHostBranchNameHint(' Fix: worktree picker + branch config! '),
getAgentHostBranchNameHint('---'),
], [
'add-agent-host-session-configuration-flow',
'fix-worktree-picker-branch-config',
undefined,
]);
});
assert.deepStrictEqual(agentHostService.createSessionCalls[0].config, config);
}));

test('handler forwards request session config via SessionConfigChanged on eager-create path', () => runWithFakedTimers({ useFakeTimers: true }, async () => {
const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables);

// Pre-seed an eagerly-created backend session so the handler
// hits the eager-create branch in `_invokeAgent` (the one that
// dispatches `SessionConfigChanged` instead of calling
// `createSession` with the config inline).
const sessionUri = AgentSession.uri('copilot', 'eager-config');
agentHostService.sessionStates.set(sessionUri.toString(), {
...createSessionState({ resource: sessionUri.toString(), provider: 'copilot', title: 'Test', status: SessionStatus.Idle, createdAt: Date.now(), modifiedAt: Date.now() }),
lifecycle: SessionLifecycle.Ready,
turns: [],
});

const sessionResource = URI.from({ scheme: 'agent-host-copilot', path: '/eager-config' });
const chatSession = await sessionHandler.provideChatSessionContent(sessionResource, CancellationToken.None);
disposables.add(toDisposable(() => chatSession.dispose()));

agentHostService.dispatchedActions.length = 0;

const registered = chatAgentService.registeredAgents.get('agent-host-copilot')!;
const config = { isolation: 'worktree', branch: 'main' };
const turnPromise = registered.impl.invoke(
makeRequest({ message: 'Fix worktree branch hint propagation', sessionResource, agentHostSessionConfig: config }),
() => { }, [], CancellationToken.None,
);
await timeout(10);
const turnDispatch = agentHostService.turnActions[0];
const turnAction = turnDispatch.action as ITurnStartedAction;
agentHostService.fireAction({ action: turnDispatch.action, serverSeq: 1, origin: { clientId: agentHostService.clientId, clientSeq: turnDispatch.clientSeq } });
agentHostService.fireAction({ action: { type: 'session/turnComplete', session: turnAction.session, turnId: turnAction.turnId } as SessionAction, serverSeq: 2, origin: undefined });
await turnPromise;

const configChanged = agentHostService.dispatchedActions.find(d => d.action.type === ActionType.SessionConfigChanged);
assert.ok(configChanged, 'expected a SessionConfigChanged dispatch');
assert.deepStrictEqual((configChanged!.action as { config: Record<string, unknown> }).config, config);
}));

test('handler uses registered working directory resolver', () => runWithFakedTimers({ useFakeTimers: true }, async () => {
const resolvedWorkingDirectory = URI.file('/resolved/working/dir');
Expand Down
Loading