diff --git a/src/vs/platform/agentHost/common/sessionConfigKeys.ts b/src/vs/platform/agentHost/common/sessionConfigKeys.ts index 33afd3210c59c1..f241039f377727 100644 --- a/src/vs/platform/agentHost/common/sessionConfigKeys.ts +++ b/src/vs/platform/agentHost/common/sessionConfigKeys.ts @@ -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', } diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index b508666d6ac026..97d4744922a94d 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -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; +} + /** * Builds the localized "Created isolated worktree for branch X" markdown * shown at the top of the first response in worktree-isolated sessions. @@ -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 { + private async _materializeProvisional(sessionId: string, prompt: string): Promise { const provisional = this._provisionalSessions.get(sessionId); if (!provisional) { throw new Error(`Cannot materialize unknown provisional session: ${sessionId}`); @@ -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); @@ -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); } @@ -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 { + protected async _resolveSessionWorkingDirectory(config: IAgentCreateSessionConfig | undefined, sessionId: string, prompt?: string): Promise { if (config?.config?.isolation !== 'worktree' || !config.workingDirectory || typeof config.config.branch !== 'string') { return config?.workingDirectory; } @@ -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 }); diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 5a4bbc60c0f5f8..365fd0447cce32 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -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'; @@ -273,8 +273,8 @@ class TestableCopilotAgent extends CopilotAgent { return stub; } - resolveWorktreeForTest(config: Parameters[0], sessionId: string): Promise { - return this._resolveSessionWorkingDirectory(config, sessionId); + resolveWorktreeForTest(config: Parameters[0], sessionId: string, prompt?: string): Promise { + return this._resolveSessionWorkingDirectory(config, sessionId, prompt); } } @@ -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 { @@ -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); @@ -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 @@ -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); @@ -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. @@ -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 }); diff --git a/src/vs/sessions/contrib/chat/browser/agentHost/agentHostSessionConfigPicker.ts b/src/vs/sessions/contrib/chat/browser/agentHost/agentHostSessionConfigPicker.ts index cbadb311f345d4..d4f7f78456bcb8 100644 --- a/src/vs/sessions/contrib/chat/browser/agentHost/agentHostSessionConfigPicker.ts +++ b/src/vs/sessions/contrib/chat/browser/agentHost/agentHostSessionConfigPicker.ts @@ -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`. diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts index d572ec36bab300..009d27099fe893 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts @@ -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'; /** diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts index 34bbc08bad979d..905f517e2f4613 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -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'; @@ -336,19 +335,6 @@ export interface IAgentHostSessionHandlerConfig { readonly customizations?: IObservable; } -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 @@ -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(); @@ -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 @@ -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, branchNameHint?: string): Promise { - 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): Promise { const workingDirectory = this._resolveRequestedWorkingDirectory(sessionResource); const requestedSession = fork ? undefined : this._resolveSessionUri(sessionResource); diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts index 4bf0dc0135edbf..d782afe38a5235 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts @@ -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'; @@ -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'; @@ -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 }).config, config); + })); test('handler uses registered working directory resolver', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const resolvedWorkingDirectory = URI.file('/resolved/working/dir');