diff --git a/src/vs/platform/agentHost/common/sessionConfigKeys.ts b/src/vs/platform/agentHost/common/sessionConfigKeys.ts index e11babc4d40..b7373c4c2c2 100644 --- a/src/vs/platform/agentHost/common/sessionConfigKeys.ts +++ b/src/vs/platform/agentHost/common/sessionConfigKeys.ts @@ -33,6 +33,8 @@ export const enum SessionConfigKey { WorktreeIncludeFiles = 'worktreeIncludeFiles', /** `'worktreeBranchTrack'` — host-owned branch tracking preference for programmatic session creation. */ WorktreeBranchTrack = 'worktreeBranchTrack', + /** `'worktreeCreateNewBranch'` — host-owned choice to create a branch instead of checking out the selected branch. */ + WorktreeCreateNewBranch = 'worktreeCreateNewBranch', /** `'agentMerge'` — client-owned Agent Merge enablement and session overrides. */ AgentMerge = 'agentMerge', /** `'agentMerge.controller'` — host-owned Agent Merge lifecycle state. */ diff --git a/src/vs/platform/agentHost/node/agentHostChangesetService.ts b/src/vs/platform/agentHost/node/agentHostChangesetService.ts index e29b9d17b14..b875b508f47 100644 --- a/src/vs/platform/agentHost/node/agentHostChangesetService.ts +++ b/src/vs/platform/agentHost/node/agentHostChangesetService.ts @@ -215,7 +215,8 @@ export class AgentHostChangesetService extends Disposable implements IAgentHostC } private _hasWorkingDirectory(session: ProtocolURI): boolean { - return !!this._configurationService.getEffectiveWorkingDirectories(session)?.[0]; + return !this._configurationService.isWorkingDirectoryPending(session) + && !!this._configurationService.getEffectiveWorkingDirectories(session)?.[0]; } registerStaticChangesets(session: ProtocolURI): void { diff --git a/src/vs/platform/agentHost/node/agentHostGitStateService.ts b/src/vs/platform/agentHost/node/agentHostGitStateService.ts index fff3b2b5eda..94f7b374053 100644 --- a/src/vs/platform/agentHost/node/agentHostGitStateService.ts +++ b/src/vs/platform/agentHost/node/agentHostGitStateService.ts @@ -338,8 +338,10 @@ export class AgentHostGitStateService extends Disposable implements IAgentHostGi async resolveSessionBaseBranchName(sessionKey: string): Promise { const state = this._stateManager.getSessionState(sessionKey); - const configuredBranch = state?.config?.values[SessionConfigKey.Isolation] === 'worktree' - ? state.config.values[SessionConfigKey.Branch] + const configValues = state?.config?.values; + const configuredBranch = configValues?.[SessionConfigKey.Isolation] === 'worktree' + && configValues[SessionConfigKey.WorktreeCreateNewBranch] !== false + ? configValues[SessionConfigKey.Branch] : undefined; if (typeof configuredBranch === 'string' && configuredBranch.trim()) { return resolveDiffBaseBranchName(configuredBranch.trim(), undefined); diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index 241ae294e7b..4b5c72a560c 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -140,6 +140,7 @@ const HOST_OWNED_SESSION_CONFIG_KEYS = [ SessionConfigKey.WorktreeBranchPrefix, SessionConfigKey.WorktreeIncludeFiles, SessionConfigKey.WorktreeBranchTrack, + SessionConfigKey.WorktreeCreateNewBranch, ] as const; /** @@ -2689,8 +2690,10 @@ export class AgentService extends Disposable implements IAgentService { } } - const workingDirectory = created.resolvedWorkingDirectory ?? config?.workingDirectories?.[0]; - void this._gitStateService.refreshSessionGitState(session.toString(), workingDirectory); + if (!this._configurationService.isWorkingDirectoryPending(session.toString())) { + const workingDirectory = created.resolvedWorkingDirectory ?? config?.workingDirectories?.[0]; + void this._gitStateService.refreshSessionGitState(session.toString(), workingDirectory); + } return session; } @@ -3643,6 +3646,9 @@ export class AgentService extends Disposable implements IAgentService { if (iso.worktreeBranchTrackProperty) { properties[SessionConfigKey.WorktreeBranchTrack] = iso.worktreeBranchTrackProperty.protocol; } + if (iso.worktreeCreateNewBranchProperty) { + properties[SessionConfigKey.WorktreeCreateNewBranch] = iso.worktreeCreateNewBranchProperty.protocol; + } if (iso.worktreeIncludeFilesProperty) { properties[SessionConfigKey.WorktreeIncludeFiles] = iso.worktreeIncludeFilesProperty.protocol; } @@ -3657,6 +3663,9 @@ export class AgentService extends Disposable implements IAgentService { if (iso.worktreeBranchTrackProperty && typeof params.config?.[SessionConfigKey.WorktreeBranchTrack] === 'boolean') { values[SessionConfigKey.WorktreeBranchTrack] = params.config[SessionConfigKey.WorktreeBranchTrack]; } + if (iso.worktreeCreateNewBranchProperty && typeof params.config?.[SessionConfigKey.WorktreeCreateNewBranch] === 'boolean') { + values[SessionConfigKey.WorktreeCreateNewBranch] = params.config[SessionConfigKey.WorktreeCreateNewBranch]; + } if (iso.worktreeIncludeFilesProperty && Array.isArray(params.config?.[SessionConfigKey.WorktreeIncludeFiles]) && params.config[SessionConfigKey.WorktreeIncludeFiles].every(pattern => typeof pattern === 'string')) { diff --git a/src/vs/platform/agentHost/node/shared/artifactServerTools.ts b/src/vs/platform/agentHost/node/shared/artifactServerTools.ts index ce14e16bf96..70ec357ff4c 100644 --- a/src/vs/platform/agentHost/node/shared/artifactServerTools.ts +++ b/src/vs/platform/agentHost/node/shared/artifactServerTools.ts @@ -45,7 +45,7 @@ export const artifactServerToolDefinitions: ToolDefinition[] = [ { name: ArtifactServerToolName.AddArtifact, title: 'Add Artifact', - description: 'Record something the user will want to open — a pull request, issue, notable commit, website, file or other resource — so it is surfaced next to the chat input.', + description: 'Record something the user will want to open — a pull request, issue, commit found while investigating or answering a question, website, file or other resource — so it is surfaced next to the chat input. Do not record commits you create unless the user explicitly asks you to add them as artifacts.', inputSchema: addArtifactInputSchema, annotations: { readOnlyHint: false }, }, @@ -172,4 +172,4 @@ export function createArtifactServerToolGroup(accessor?: IArtifactServerToolAcce * The instruction appended to every agent's host instructions while the * artifact tools are enabled. */ -export const ARTIFACT_TOOLS_INSTRUCTION = `When you produce something the user will want to open — a pull request, an issue, a notable commit, a website, a plan file or another resource — record it once with \`${ArtifactServerToolName.AddArtifact}\` (types: ${SESSION_ARTIFACT_TYPES.join(', ')}; use \`${SessionArtifactType.Resource}\` when nothing else fits). Do not record routine files you merely edited, and do not record every commit you make — record a commit only when the user asked you to commit, or when you found a commit worth showing them, for example while investigating.`; +export const ARTIFACT_TOOLS_INSTRUCTION = `When you produce something the user will want to open — a pull request, an issue, a website, a plan file or another resource — or find a notable commit worth showing the user while investigating or answering a question, record it once with \`${ArtifactServerToolName.AddArtifact}\` (types: ${SESSION_ARTIFACT_TYPES.join(', ')}; use \`${SessionArtifactType.Resource}\` when nothing else fits). Do not record routine files you merely edited. Do not record commits you create unless the user explicitly asks you to add them as artifacts.`; diff --git a/src/vs/platform/agentHost/node/shared/worktreeIsolation.ts b/src/vs/platform/agentHost/node/shared/worktreeIsolation.ts index 414bed17323..2e1c9aef4a0 100644 --- a/src/vs/platform/agentHost/node/shared/worktreeIsolation.ts +++ b/src/vs/platform/agentHost/node/shared/worktreeIsolation.ts @@ -293,6 +293,8 @@ export interface IIsolationConfigContribution { readonly worktreeIncludeFilesProperty: ISchemaProperty | undefined; /** Read-only carrier for the programmatic worktree branch tracking preference. */ readonly worktreeBranchTrackProperty: ISchemaProperty | undefined; + /** Read-only carrier for checking out the selected branch directly. */ + readonly worktreeCreateNewBranchProperty: ISchemaProperty | undefined; readonly isolationValue: 'folder' | 'worktree'; readonly branchDefault: string | undefined; readonly branchValue: string | undefined; @@ -475,6 +477,7 @@ export class WorktreeIsolation extends Disposable implements IAgentHostWorktreeI let worktreeBranchPrefixProperty: ISchemaProperty | undefined; let worktreeIncludeFilesProperty: ISchemaProperty | undefined; let worktreeBranchTrackProperty: ISchemaProperty | undefined; + let worktreeCreateNewBranchProperty: ISchemaProperty | undefined; if (gitInfo) { const branchReadOnly = isolationValue === 'folder'; branchDefault = isolationValue === 'worktree' ? gitInfo.defaultBranch.name : gitInfo.currentBranch; @@ -520,6 +523,15 @@ export class WorktreeIsolation extends Disposable implements IAgentHostWorktreeI sessionMutable: false, }); + worktreeCreateNewBranchProperty = schemaProperty({ + type: 'boolean', + title: localize('agentHost.sessionConfig.worktreeCreateNewBranch', "Create New Worktree Branch"), + description: localize('agentHost.sessionConfig.worktreeCreateNewBranchDescription', "Whether to create a new branch for the isolated worktree."), + default: true, + readOnly: true, + sessionMutable: false, + }); + worktreeIncludeFilesProperty = schemaProperty({ type: 'array', title: localize('agentHost.sessionConfig.worktreeIncludeFiles', "Worktree Include Files"), @@ -533,7 +545,7 @@ export class WorktreeIsolation extends Disposable implements IAgentHostWorktreeI }); } - return { isolationProperty, branchProperty, worktreeBranchPrefixProperty, worktreeBranchTrackProperty, worktreeIncludeFilesProperty, isolationValue, branchDefault, branchValue }; + return { isolationProperty, branchProperty, worktreeBranchPrefixProperty, worktreeBranchTrackProperty, worktreeCreateNewBranchProperty, worktreeIncludeFilesProperty, isolationValue, branchDefault, branchValue }; } /** @@ -563,7 +575,7 @@ export class WorktreeIsolation extends Disposable implements IAgentHostWorktreeI /** * Resolves the effective working directory for a session that is about to * be materialized. When the session config selects `worktree` isolation on - * a git repository, creates a fresh branch + worktree, records it for + * a git repository, creates or checks out a branch in a worktree, records it for * cleanup, queues the first-turn announcement, persists the worktree * metadata, and returns the worktree URI. Otherwise returns the requested * working directory unchanged. @@ -591,50 +603,69 @@ export class WorktreeIsolation extends Disposable implements IAgentHostWorktreeI } const repositoryRoot = await this._resolvePrimaryWorktreeRoot(checkoutRoot, checkoutRoot); - const worktreesRoot = getWorktreesRoot(repositoryRoot); + + const selectedBranch = config[SessionConfigKey.Branch] as string; + const worktreeBranchTrack = config[SessionConfigKey.WorktreeBranchTrack] === true; + const worktreeCreateNewBranch = config[SessionConfigKey.WorktreeCreateNewBranch] !== false; + // Prefix (e.g. the user's `git.branchPrefix`) the client forwards for // worktree-isolated sessions. Prepended ahead of the built-in `agents/` // prefix when naming the branch and stripped from the worktree dir name. - const worktreeBranchPrefix = typeof config[SessionConfigKey.WorktreeBranchPrefix] === 'string' + const worktreeBranchPrefix = worktreeCreateNewBranch && typeof config[SessionConfigKey.WorktreeBranchPrefix] === 'string' ? config[SessionConfigKey.WorktreeBranchPrefix] as string : undefined; - const selectedBranch = config[SessionConfigKey.Branch] as string; - const { branchName, worktree, baseBranch } = await this._worktreeCreationSequencer.queue(repositoryRoot.toString(), async () => { - onProgress?.(buildWorktreeProgressText(WorktreeCreationPhase.NamingBranch)); - const branchName = await this._branchNameGenerator.generateBranchName({ - sessionId, - message: prompt, - githubToken, - branchPrefix: worktreeBranchPrefix, - branchNameCollides: async candidate => { - if (await this._gitService.branchExists(repositoryRoot, candidate).catch(() => true)) { - return true; - } - const candidateWorktree = URI.joinPath(worktreesRoot, getWorktreeName(candidate, worktreeBranchPrefix)); - return fileExists(candidateWorktree.fsPath); - }, - }); - const worktree = URI.joinPath(worktreesRoot, getWorktreeName(branchName, worktreeBranchPrefix)); - const baseBranch = await this._resolveBranchStartPoint(repositoryRoot, selectedBranch); - await fs.mkdir(worktreesRoot.fsPath, { recursive: true }); + + const { worktreePath, branchName, baseBranch } = await this._worktreeCreationSequencer.queue(repositoryRoot.toString(), async () => { + const worktreesRoot = getWorktreesRoot(repositoryRoot); + + if (worktreeCreateNewBranch) { + onProgress?.(buildWorktreeProgressText(WorktreeCreationPhase.NamingBranch)); + } + const newBranchName = worktreeCreateNewBranch + ? await this._branchNameGenerator.generateBranchName({ + sessionId, + message: prompt, + githubToken, + branchPrefix: worktreeBranchPrefix, + branchNameCollides: async candidate => { + if (await this._gitService.branchExists(repositoryRoot, candidate).catch(() => true)) { + return true; + } + const candidateWorktree = URI.joinPath(worktreesRoot, getWorktreeName(candidate, worktreeBranchPrefix)); + return fileExists(candidateWorktree.fsPath); + }, + }) + : undefined; + + const branchStartPoint = await this._resolveBranchStartPoint(repositoryRoot, selectedBranch); + + const baseBranch = worktreeCreateNewBranch + ? branchStartPoint + : (await this._gitService.getDefaultBranch(repositoryRoot))?.startPoint; // Git suppresses progress for the first couple of seconds, so name // the phase up front rather than leaving the label stale until the // first percentage arrives. onProgress?.(buildWorktreeProgressText(WorktreeCreationPhase.CheckingOut)); - const worktreeBranchTrack = config[SessionConfigKey.WorktreeBranchTrack] === true; + await fs.mkdir(worktreesRoot.fsPath, { recursive: true }); + const worktreePath = URI.joinPath(worktreesRoot, getWorktreeName(newBranchName ?? selectedBranch, worktreeBranchPrefix)); + await withPercentProgress(WorktreeCreationPhase.CheckingOut, onProgress, progress => this._gitService.addWorktree(repositoryRoot, { - path: worktree, - commitish: baseBranch, - newBranchName: branchName, + path: worktreePath, + commitish: worktreeCreateNewBranch + ? branchStartPoint + : selectedBranch, + newBranchName, + preferRemoteBranch: worktreeCreateNewBranch, track: worktreeBranchTrack, - preferRemoteBranch: true, onProgress: progress, })); - return { branchName, worktree, baseBranch }; + + return { branchName: newBranchName ?? selectedBranch, worktreePath, baseBranch }; }); + const worktreeIncludeFiles = Array.isArray(config[SessionConfigKey.WorktreeIncludeFiles]) && config[SessionConfigKey.WorktreeIncludeFiles].every(pattern => typeof pattern === 'string') ? config[SessionConfigKey.WorktreeIncludeFiles] as readonly string[] @@ -643,21 +674,25 @@ export class WorktreeIsolation extends Disposable implements IAgentHostWorktreeI try { onProgress?.(buildWorktreeProgressText(WorktreeCreationPhase.CopyingIncludeFiles)); await withPercentProgress(WorktreeCreationPhase.CopyingIncludeFiles, onProgress, progress => - this._gitService.copyWorktreeIncludeFiles(checkoutRoot, worktree, worktreeIncludeFiles, progress)); + this._gitService.copyWorktreeIncludeFiles(checkoutRoot, worktreePath, worktreeIncludeFiles, progress)); } catch (error) { this._logService.warn(`[${this._logLabel}:${sessionId}] Failed to copy worktree include files: ${errorMessage(error)}`); } } - this._materializedWorktrees.set(sessionId, { repositoryRoot, worktree }); + + this._materializedWorktrees.set(sessionId, { repositoryRoot, worktree: worktreePath }); + // Queue the worktree announcement so the first turn (live) and any // subsequent restore (history) both surface the message in the chat. this._pendingFirstTurnAnnouncements.set(sessionId, buildWorktreeAnnouncementText(branchName)); + try { - await this._writeWorktreeMetadata(sessionUri, { branchName, baseBranch, worktreePath: worktree, repositoryRoot }); + await this._writeWorktreeMetadata(sessionUri, { repositoryRoot, worktreePath, baseBranch, branchName }); } catch (error) { this._logService.warn(`[${this._logLabel}:${sessionId}] Failed to persist worktree branch metadata: ${errorMessage(error)}`); } - return worktree; + + return worktreePath; } /** Resolves a persisted working directory, repairing a removed worktree when possible. */ diff --git a/src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts b/src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts index a74bd772cfd..59e92517d4e 100644 --- a/src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts +++ b/src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts @@ -764,6 +764,68 @@ suite('AgentHostGitService - worktree helpers (real git)', () => { } }); + (hasGit ? test : test.skip)('addWorktree preserves tracking when attaching an existing branch', async () => { + const dir = initRepo(); + const remotePath = join(dir, 'remote.git'); + cp.execFileSync('git', ['init', '--bare', '-q', remotePath], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['remote', 'add', 'origin', remotePath], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['push', '-q', 'origin', 'main'], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['branch', 'feature'], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['push', '-q', '--set-upstream', 'origin', 'feature'], { cwd: dir, env, stdio: 'pipe' }); + const wtPath = join(dir, '..', `wt-${Date.now()}`); + try { + await svc!.addWorktree(URI.file(dir), { + path: URI.file(wtPath), + commitish: 'feature', + track: true, + }); + + assert.deepStrictEqual({ + branch: cp.execFileSync('git', ['branch', '--show-current'], { cwd: wtPath, env, encoding: 'utf8' }).trim(), + upstream: cp.execFileSync('git', ['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{upstream}'], { cwd: wtPath, env, encoding: 'utf8' }).trim(), + }, { + branch: 'feature', + upstream: 'origin/feature', + }); + } finally { + try { await svc!.removeWorktree(URI.file(dir), URI.file(wtPath), { force: true }); } catch { /* best-effort cleanup */ } + rmDirWithRetry(wtPath); + } + }); + + (hasGit ? test : test.skip)('addWorktree automatically tracks a remote branch when creating its local branch', async () => { + const dir = initRepo(); + const remotePath = join(dir, 'remote.git'); + cp.execFileSync('git', ['init', '--bare', '-q', remotePath], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['remote', 'add', 'origin', remotePath], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['checkout', '-q', '-b', 'feature'], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['commit', '-q', '--allow-empty', '-m', 'feature'], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['push', '-q', 'origin', 'feature'], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['checkout', '-q', 'main'], { cwd: dir, env, stdio: 'pipe' }); + cp.execFileSync('git', ['branch', '-D', 'feature'], { cwd: dir, env, stdio: 'pipe' }); + const wtPath = join(dir, '..', `wt-${Date.now()}`); + try { + await svc!.addWorktree(URI.file(dir), { + path: URI.file(wtPath), + commitish: 'feature', + newBranchName: 'feature', + track: true, + preferRemoteBranch: true, + }); + + assert.deepStrictEqual({ + branch: cp.execFileSync('git', ['branch', '--show-current'], { cwd: wtPath, env, encoding: 'utf8' }).trim(), + upstream: cp.execFileSync('git', ['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{upstream}'], { cwd: wtPath, env, encoding: 'utf8' }).trim(), + }, { + branch: 'feature', + upstream: 'origin/feature', + }); + } finally { + try { await svc!.removeWorktree(URI.file(dir), URI.file(wtPath), { force: true }); } catch { /* best-effort cleanup */ } + rmDirWithRetry(wtPath); + } + }); + (hasGit ? test : test.skip)('removeWorktree preserves dirty work unless forced', async () => { const dir = initRepo(); const fs = await import('fs/promises'); diff --git a/src/vs/platform/agentHost/test/node/agentHostGitStateService.test.ts b/src/vs/platform/agentHost/test/node/agentHostGitStateService.test.ts index 84fed362bde..0e46fd32959 100644 --- a/src/vs/platform/agentHost/test/node/agentHostGitStateService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostGitStateService.test.ts @@ -210,7 +210,7 @@ suite('AgentHostGitStateService', () => { }; } - function seedSession(stateManager: AgentHostStateManager, options?: { workingDirectory?: string; project?: string; gitState?: ISessionGitState; gitHubState?: ISessionGitHubState; isolation?: 'folder' | 'worktree'; baseBranch?: string; createdAt?: number }): void { + function seedSession(stateManager: AgentHostStateManager, options?: { workingDirectory?: string; project?: string; gitState?: ISessionGitState; gitHubState?: ISessionGitHubState; isolation?: 'folder' | 'worktree'; baseBranch?: string; createNewBranch?: boolean; createdAt?: number }): void { const summary: SessionSummary = { resource: SESSION, provider: 'mock', @@ -230,6 +230,7 @@ suite('AgentHostGitStateService', () => { values: { [SessionConfigKey.Isolation]: options.isolation, ...(options.baseBranch ? { [SessionConfigKey.Branch]: options.baseBranch } : {}), + ...(options.createNewBranch !== undefined ? { [SessionConfigKey.WorktreeCreateNewBranch]: options.createNewBranch } : {}), }, }); } @@ -314,6 +315,23 @@ suite('AgentHostGitStateService', () => { assert.deepStrictEqual(h.gitBaseBranches, ['release']); })); + test('uses the persisted base branch when the selected branch is checked out directly', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const h = createHarness(); + seedSession(h.stateManager, { + workingDirectory: WORKING_DIRECTORY, + project: 'file:///repo', + isolation: 'worktree', + baseBranch: 'feature/pr', + createNewBranch: false, + }); + await h.db.setMetadata(META_DIFF_BASE_BRANCH, 'origin/main'); + h.setGitResult({ branchName: 'feature/pr', baseBranchName: 'main' }); + + await h.service.refreshSessionGitState(SESSION, undefined); + + assert.deepStrictEqual(h.gitBaseBranches, ['main']); + })); + test('uses the persisted worktree base branch for an adopted linked worktree', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const h = createHarness(); seedSession(h.stateManager, { diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index bd3d854d6ea..9b5136b6f4b 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -36,6 +36,7 @@ import { ClaudeSessionConfigKey } from '../../common/claudeSessionConfigKeys.js' import { CodexSessionConfigKey } from '../../common/codexSessionConfigKeys.js'; import { ISessionDatabase, ISessionDataService } from '../../common/sessionDataService.js'; import { META_GITHUB_STATE, META_SOURCE_CONTROL_STATE } from '../../common/agentHostGitStateService.js'; +import { GitRefType } from '../../common/agentHostGitService.js'; import { SessionConfigKey } from '../../common/sessionConfigKeys.js'; import { AgentMergeConfigKey, readAgentMergeSessionState } from '../../common/agentMerge.js'; import { SessionDatabase } from '../../node/sessionDatabase.js'; @@ -783,6 +784,7 @@ suite('AgentService (node dispatcher)', () => { [SessionConfigKey.WorktreeBranchPrefix]: 'users/test/', [SessionConfigKey.WorktreeIncludeFiles]: ['.env'], [SessionConfigKey.WorktreeBranchTrack]: false, + [SessionConfigKey.WorktreeCreateNewBranch]: false, providerSetting: 'selected', }, }); @@ -800,6 +802,7 @@ suite('AgentService (node dispatcher)', () => { [SessionConfigKey.WorktreeBranchPrefix]: 'users/test/', [SessionConfigKey.WorktreeIncludeFiles]: ['.env'], [SessionConfigKey.WorktreeBranchTrack]: false, + [SessionConfigKey.WorktreeCreateNewBranch]: false, providerSetting: 'completion', }, property: 'providerSetting', @@ -822,6 +825,7 @@ suite('AgentService (node dispatcher)', () => { branchPrefix: selected.values[SessionConfigKey.WorktreeBranchPrefix], includeFiles: selected.values[SessionConfigKey.WorktreeIncludeFiles], branchTrack: selected.values[SessionConfigKey.WorktreeBranchTrack], + createNewBranch: selected.values[SessionConfigKey.WorktreeCreateNewBranch], providerSetting: selected.values.providerSetting, }, folder: { @@ -844,7 +848,7 @@ suite('AgentService (node dispatcher)', () => { agentMergeController: { lastPromptFingerprint: 'fingerprint' }, providerSetting: 'initial', }, - selected: { isolation: 'worktree', branch: 'feature/config', branchPrefix: 'users/test/', includeFiles: ['.env'], branchTrack: false, providerSetting: 'selected' }, + selected: { isolation: 'worktree', branch: 'feature/config', branchPrefix: 'users/test/', includeFiles: ['.env'], branchTrack: false, createNewBranch: false, providerSetting: 'selected' }, folder: { isolation: 'folder', branch: 'feature', providerSetting: 'folder' }, }); }); @@ -12341,6 +12345,107 @@ suite('AgentService (node dispatcher)', () => { assert.strictEqual(state?.workingDirectories?.[0], worktreeDir.toString()); }); + test('pending worktree session defers git state and branch changes until materialization', async () => { + class ProvisionalWorktreeAgent extends MockAgent { + private readonly _onDidMaterializeChat = new Emitter(); + override readonly onDidMaterializeChat = this._onDidMaterializeChat.event; + override readonly chats: IAgentChats = withChatOverrides(getChatSurface(this), base => ({ + createChat: (chat, context, options) => createProvisionalChat(base, chat, context, options), + })); + + materialize(session: URI, workingDirectory: URI): void { + this._onDidMaterializeChat.fire({ + chat: URI.parse(buildDefaultChatUri(session)), + workingDirectories: [workingDirectory], + project: undefined, + }); + } + + override dispose(): void { + this._onDidMaterializeChat.dispose(); + super.dispose(); + } + } + + const sourceDir = URI.file('/source/repo'); + const worktreeDir = URI.file('/source/repo.worktrees/feature'); + const gitStateCalls: Array<{ resource: string; baseBranch: string | undefined }> = []; + const diffCalls: string[] = []; + const gitService = createNoopGitService(); + gitService.getRepositoryRoot = async () => sourceDir; + gitService.revParse = async () => 'head'; + gitService.getDefaultBranch = async () => ({ name: 'main', startPoint: 'origin/main' }); + gitService.getCurrentBranch = async () => 'main'; + gitService.getBranches = async () => [{ ref: 'refs/heads/main', name: 'main', kind: GitRefType.Head }]; + gitService.getSessionGitState = async (resource, baseBranch) => { + gitStateCalls.push({ resource: resource.toString(), baseBranch }); + return { branchName: 'feature', baseBranchName: 'main' }; + }; + gitService.computeSessionFileDiffs = async resource => { + diffCalls.push(resource.toString()); + return []; + }; + + const localService = disposables.add(createTestAgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + const isolation = disposables.add(new WorktreeIsolation( + { generateBranchName: async () => { throw new Error('should not generate a branch'); } }, + gitService, + new TestCopilotApiService(), + nullSessionDataService, + new NullLogService(), + )); + localService.setWorktreeIsolation(isolation); + const agent = new ProvisionalWorktreeAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + localService.registerProvider(agent); + + const session = await localService.createSession({ + provider: agent.id, + workingDirectories: [sourceDir], + config: { + [SessionConfigKey.Isolation]: 'worktree', + [SessionConfigKey.Branch]: 'feature', + [SessionConfigKey.WorktreeCreateNewBranch]: false, + }, + }); + const branchChangeset = buildBranchChangesetUri(session.toString()); + localService.addSubscriber(URI.parse(branchChangeset), 'client-1'); + await timeout(0); + + const beforeMaterialization = { + workingDirectory: localService.stateManager.getSessionState(session.toString())?.workingDirectories?.[0], + gitStateCalls: [...gitStateCalls], + diffCalls: [...diffCalls], + }; + + isolation.clearPending(AgentSession.id(session)); + agent.materialize(session, worktreeDir); + for (let i = 0; i < 20 && (gitStateCalls.length === 0 || diffCalls.length === 0); i++) { + await timeout(0); + } + + assert.deepStrictEqual({ + beforeMaterialization, + afterMaterialization: { + workingDirectory: localService.stateManager.getSessionState(session.toString())?.workingDirectories?.[0], + gitStateCalls, + diffCalls: [...new Set(diffCalls)], + }, + }, { + beforeMaterialization: { + workingDirectory: sourceDir.toString(), + gitStateCalls: [], + diffCalls: [], + }, + afterMaterialization: { + workingDirectory: worktreeDir.toString(), + gitStateCalls: [{ resource: worktreeDir.toString(), baseBranch: undefined }], + diffCalls: [worktreeDir.toString()], + }, + }); + localService.unsubscribe(URI.parse(branchChangeset), 'client-1'); + }); + test('_resolveWorkingDirectoryBeforeSend returns the full set (index 0 + tail), or undefined when unset', async () => { const resolver = service as unknown as { _resolveWorkingDirectoryBeforeSend: (p: { session: string; chat: string; turnId: string; prompt: string }) => Promise; diff --git a/src/vs/platform/agentHost/test/node/shared/worktreeIsolation.test.ts b/src/vs/platform/agentHost/test/node/shared/worktreeIsolation.test.ts index f4e59d44249..3ec01b3e73c 100644 --- a/src/vs/platform/agentHost/test/node/shared/worktreeIsolation.test.ts +++ b/src/vs/platform/agentHost/test/node/shared/worktreeIsolation.test.ts @@ -152,17 +152,17 @@ suite('WorktreeIsolation', () => { const noCommits = await isolation.resolveIsolationConfig({ workingDirectory: repoRoot, config: undefined }); assert.deepStrictEqual({ - noRepo: { enum: noRepo.isolationProperty.protocol.enum, value: noRepo.isolationValue, branch: noRepo.branchProperty, prefix: noRepo.worktreeBranchPrefixProperty, includeFiles: noRepo.worktreeIncludeFilesProperty, branchTrack: noRepo.worktreeBranchTrackProperty }, - repoWorktree: { enum: repoWorktree.isolationProperty.protocol.enum, value: repoWorktree.isolationValue, branchDefault: repoWorktree.branchDefault, branchReadOnly: repoWorktree.branchProperty?.protocol.readOnly, prefixReadOnly: repoWorktree.worktreeBranchPrefixProperty?.protocol.readOnly, includeFilesReadOnly: repoWorktree.worktreeIncludeFilesProperty?.protocol.readOnly, branchTrackReadOnly: repoWorktree.worktreeBranchTrackProperty?.protocol.readOnly }, + noRepo: { enum: noRepo.isolationProperty.protocol.enum, value: noRepo.isolationValue, branch: noRepo.branchProperty, prefix: noRepo.worktreeBranchPrefixProperty, includeFiles: noRepo.worktreeIncludeFilesProperty, branchTrack: noRepo.worktreeBranchTrackProperty, createNewBranch: noRepo.worktreeCreateNewBranchProperty }, + repoWorktree: { enum: repoWorktree.isolationProperty.protocol.enum, value: repoWorktree.isolationValue, branchDefault: repoWorktree.branchDefault, branchReadOnly: repoWorktree.branchProperty?.protocol.readOnly, prefixReadOnly: repoWorktree.worktreeBranchPrefixProperty?.protocol.readOnly, includeFilesReadOnly: repoWorktree.worktreeIncludeFilesProperty?.protocol.readOnly, branchTrackReadOnly: repoWorktree.worktreeBranchTrackProperty?.protocol.readOnly, createNewBranchReadOnly: repoWorktree.worktreeCreateNewBranchProperty?.protocol.readOnly }, repoWorktreeSelected: { branchDefault: repoWorktreeSelected.branchDefault, branchValue: repoWorktreeSelected.branchValue, branchEnum: repoWorktreeSelected.branchProperty?.protocol.enum }, - repoFolder: { value: repoFolder.isolationValue, branchDefault: repoFolder.branchDefault, branchReadOnly: repoFolder.branchProperty?.protocol.readOnly, hasPrefix: !!repoFolder.worktreeBranchPrefixProperty, hasIncludeFiles: !!repoFolder.worktreeIncludeFilesProperty, hasBranchTrack: !!repoFolder.worktreeBranchTrackProperty }, - noCommits: { enum: noCommits.isolationProperty.protocol.enum, value: noCommits.isolationValue, branch: noCommits.branchProperty, prefix: noCommits.worktreeBranchPrefixProperty, includeFiles: noCommits.worktreeIncludeFilesProperty, branchTrack: noCommits.worktreeBranchTrackProperty }, + repoFolder: { value: repoFolder.isolationValue, branchDefault: repoFolder.branchDefault, branchReadOnly: repoFolder.branchProperty?.protocol.readOnly, hasPrefix: !!repoFolder.worktreeBranchPrefixProperty, hasIncludeFiles: !!repoFolder.worktreeIncludeFilesProperty, hasBranchTrack: !!repoFolder.worktreeBranchTrackProperty, hasCreateNewBranch: !!repoFolder.worktreeCreateNewBranchProperty }, + noCommits: { enum: noCommits.isolationProperty.protocol.enum, value: noCommits.isolationValue, branch: noCommits.branchProperty, prefix: noCommits.worktreeBranchPrefixProperty, includeFiles: noCommits.worktreeIncludeFilesProperty, branchTrack: noCommits.worktreeBranchTrackProperty, createNewBranch: noCommits.worktreeCreateNewBranchProperty }, }, { - noRepo: { enum: ['folder'], value: 'folder', branch: undefined, prefix: undefined, includeFiles: undefined, branchTrack: undefined }, - repoWorktree: { enum: ['folder', 'worktree'], value: 'worktree', branchDefault: 'main', branchReadOnly: false, prefixReadOnly: true, includeFilesReadOnly: true, branchTrackReadOnly: true }, + noRepo: { enum: ['folder'], value: 'folder', branch: undefined, prefix: undefined, includeFiles: undefined, branchTrack: undefined, createNewBranch: undefined }, + repoWorktree: { enum: ['folder', 'worktree'], value: 'worktree', branchDefault: 'main', branchReadOnly: false, prefixReadOnly: true, includeFilesReadOnly: true, branchTrackReadOnly: true, createNewBranchReadOnly: true }, repoWorktreeSelected: { branchDefault: 'main', branchValue: 'feature', branchEnum: ['main'] }, - repoFolder: { value: 'folder', branchDefault: 'feature', branchReadOnly: true, hasPrefix: true, hasIncludeFiles: true, hasBranchTrack: true }, - noCommits: { enum: ['folder'], value: 'folder', branch: undefined, prefix: undefined, includeFiles: undefined, branchTrack: undefined }, + repoFolder: { value: 'folder', branchDefault: 'feature', branchReadOnly: true, hasPrefix: true, hasIncludeFiles: true, hasBranchTrack: true, hasCreateNewBranch: true }, + noCommits: { enum: ['folder'], value: 'folder', branch: undefined, prefix: undefined, includeFiles: undefined, branchTrack: undefined, createNewBranch: undefined }, }); }); @@ -205,6 +205,49 @@ suite('WorktreeIsolation', () => { }); }); + test('checks out an existing selected branch and uses the default branch as the diff base', async () => { + const gitService = createGitService(); + gitService.getDefaultBranch = async () => ({ name: 'main', startPoint: 'origin/main' }); + const isolation = createIsolation(disposables, { + gitService, + branchNameGenerator: { generateBranchName: async () => { throw new Error('should not generate a branch'); } }, + }); + + const worktree = await isolation.resolveWorkingDirectory({ + sessionUri, + sessionId, + workingDirectory: repoRoot, + config: { + [SessionConfigKey.Isolation]: 'worktree', + [SessionConfigKey.Branch]: 'feature', + [SessionConfigKey.WorktreeBranchTrack]: true, + [SessionConfigKey.WorktreeCreateNewBranch]: false, + }, + }); + + assert.deepStrictEqual({ + worktree: worktree?.toString(), + addWorktreeArgs: addWorktreeCalls.map(call => ({ + commitish: call.commitish, + newBranchName: call.newBranchName, + track: call.track, + preferRemoteBranch: call.preferRemoteBranch, + })), + branchName: await db.getMetadata('copilot.worktree.branchName'), + diffBaseBranch: await db.getMetadata('agentHost.diffBaseBranch'), + }, { + worktree: URI.joinPath(worktreesRoot, 'feature').toString(), + addWorktreeArgs: [{ + commitish: 'feature', + newBranchName: undefined, + track: true, + preferRemoteBranch: false, + }], + branchName: 'feature', + diffBaseBranch: 'origin/main', + }); + }); + test('resolveWorkingDirectory creates a worktree, persists metadata, queues the announcement, and is idempotent', async () => { const isolation = createIsolation(disposables); const config = { [SessionConfigKey.Isolation]: 'worktree', [SessionConfigKey.Branch]: 'main' }; diff --git a/src/vs/sessions/browser/parts/sessionView.ts b/src/vs/sessions/browser/parts/sessionView.ts index bb178ce3ab2..77bbb0b6299 100644 --- a/src/vs/sessions/browser/parts/sessionView.ts +++ b/src/vs/sessions/browser/parts/sessionView.ts @@ -22,6 +22,7 @@ import { autorun, observableValue } from '../../../base/common/observable.js'; import { SessionIsMaximizedContext } from '../../common/contextkeys.js'; import { AGENTS_CENTERED_CONTENT_MAX_WIDTH } from '../../common/layoutConstants.js'; import { setActiveSessionContextKeys } from '../../services/sessions/common/sessionContextKeys.js'; +import { ISessionChangesStatsCache } from '../../services/sessions/common/sessionChangesStatsCache.js'; import { applySessionViewThemeColors } from './sessionBarStyles.js'; import { IChatViewFactory } from '../../services/chatView/browser/chatViewFactory.js'; @@ -89,6 +90,7 @@ export class SessionView extends Disposable implements ISerializableView { @IInstantiationService instantiationService: IInstantiationService, @IContextKeyService contextKeyService: IContextKeyService, @IThemeService private readonly themeService: IThemeService, + @ISessionChangesStatsCache private readonly _changesStatsCache: ISessionChangesStatsCache, ) { super(); @@ -193,7 +195,7 @@ export class SessionView extends Disposable implements ISerializableView { // scoped service whenever the session's observable properties change. // Passing `undefined` resets the keys to their defaults. return autorun(reader => { - setActiveSessionContextKeys(session, this._scopedContextKeyService, reader); + setActiveSessionContextKeys(session, this._scopedContextKeyService, reader, this._changesStatsCache); }); } diff --git a/src/vs/sessions/common/contextkeys.ts b/src/vs/sessions/common/contextkeys.ts index eb15ba98562..e5514cc3183 100644 --- a/src/vs/sessions/common/contextkeys.ts +++ b/src/vs/sessions/common/contextkeys.ts @@ -40,6 +40,7 @@ export const SessionIsReadContext = new RawContextKey('sessionIsRead', export const SessionIsArchivedContext = new RawContextKey('sessionIsArchived', false, localize('sessionIsArchived', "Whether the session in scope is archived/marked as done (the active session globally, or a specific session within an isolated component such as the session view or a context menu overlay)")); export const SessionIsActiveContext = new RawContextKey('sessionIsActive', false, localize('sessionIsActive', "Whether the session in scope is in progress or needs input")); export const SessionHasChangesContext = new RawContextKey('sessionHasChanges', false, localize('sessionHasChanges', "Whether the session view's session has pending changes (insertions or deletions)")); +export const SessionHasCachedChangesContext = new RawContextKey('sessionHasCachedChanges', false, localize('sessionHasCachedChanges', "Whether the session view's session has remembered changes from the last time its changes pill was shown, while it has not reported its own changes yet. Used to render the changes pill optimistically when a session opens")); export const SessionHasPullRequestContext = new RawContextKey('sessionHasPullRequest', false, localize('sessionHasPullRequest', "Whether the session view's session is associated with a GitHub pull request")); export const SessionHasIssuesContext = new RawContextKey('sessionHasIssues', false, localize('sessionHasIssues', "Whether the session view's session references at least one GitHub issue")); export const SessionHasWorkspaceContext = new RawContextKey('sessionHasWorkspace', false, localize('sessionHasWorkspace', "Whether the session view's session has an associated workspace folder")); diff --git a/src/vs/sessions/contrib/changes/browser/changesActions.ts b/src/vs/sessions/contrib/changes/browser/changesActions.ts index 4a6415872ea..af4995d32cf 100644 --- a/src/vs/sessions/contrib/changes/browser/changesActions.ts +++ b/src/vs/sessions/contrib/changes/browser/changesActions.ts @@ -27,10 +27,11 @@ import { DiffEditorWidget } from '../../../../editor/browser/widget/diffEditor/d import { IAgentWorkbenchLayoutService } from '../../../browser/workbench.js'; import { Menus } from '../../../browser/menus.js'; import { ChatPillActionViewItem } from '../../../../workbench/browser/chatPills.js'; -import { IsQuickChatSessionContext, SessionHasChangesContext, SinglePaneLayoutEnabledContext } from '../../../common/contextkeys.js'; +import { IsQuickChatSessionContext, SessionHasCachedChangesContext, SessionHasChangesContext, SinglePaneLayoutEnabledContext } from '../../../common/contextkeys.js'; import { ISessionContext } from '../../../services/sessions/browser/sessionContext.js'; import { ISessionsService } from '../../../services/sessions/browser/sessionsService.js'; import { SessionChangesetOperationScope } from '../../../services/sessions/common/session.js'; +import { ISessionChangesStatsCache, readSessionChangesStats } from '../../../services/sessions/common/sessionChangesStatsCache.js'; import { IActiveSession } from '../../../services/sessions/common/sessionsManagement.js'; import { IChangesViewService } from '../common/changesViewService.js'; import { ChangesMultiDiffSourceResolver, SessionChangesReviewedFilesContext } from './changesMultiDiffSourceResolver.js'; @@ -49,13 +50,14 @@ class ViewAllChangesAction extends Action2 { title: localize2('agentSessions.changes', 'Changes'), icon: Codicon.diffMultiple, f1: false, - // Metadata pill rendered with live +/- counts. + // Metadata pill rendered with live +/- counts, or the counts last shown + // for the session while it has not reported its changes yet. menu: { id: Menus.SessionHeaderMeta, group: 'navigation', order: 0, when: ContextKeyExpr.and( - SessionHasChangesContext, + ContextKeyExpr.or(SessionHasChangesContext, SessionHasCachedChangesContext), ContextKeyExpr.or(IsQuickChatSessionContext.negate(), SinglePaneLayoutEnabledContext) ) }, @@ -240,6 +242,10 @@ interface IDiffStats { * session's {@link ISession.changesSummary} when available, falling back to aggregating the * changeset the provider marks as {@link ISessionChangeset.isDefault} (or the session's * top-level {@link IActiveSession.changes} when none is default). + * + * A session reports its changes late, so until it reports any the counts last shown for it + * are taken from the {@link ISessionChangesStatsCache} — the pill is then already there, + * with plausible counts, the moment the session opens. */ export class ViewAllChangesActionViewItem extends ChatPillActionViewItem { @@ -249,6 +255,7 @@ export class ViewAllChangesActionViewItem extends ChatPillActionViewItem { action: MenuItemAction, options: IActionViewItemOptions, @ISessionContext sessionContext: ISessionContext, + @ISessionChangesStatsCache changesStatsCache: ISessionChangesStatsCache, ) { super(undefined, action, options); @@ -257,33 +264,15 @@ export class ViewAllChangesActionViewItem extends ChatPillActionViewItem { const workspace = session?.workspace.read(reader); const branch = workspace?.folders[0]?.gitRepository?.branchName?.trim(); - // Prefer the provider-supplied changes summary which reflects the - // session's authoritative aggregate. Fall back to aggregating the - // default changeset's changes when no summary is available. - const changesSummary = session?.changesSummary?.read(reader); - if (changesSummary) { - return { - branch, - files: changesSummary.files, - insertions: changesSummary.additions, - deletions: changesSummary.deletions, - } satisfies IDiffStats; - } - - const defaultChangeset = session?.changesets.read(reader)?.find(c => c.isDefault.read(reader)); - const changes = (defaultChangeset?.changes.read(reader) ?? session?.changes.read(reader)) ?? []; - - let insertions = 0, deletions = 0; - for (const change of changes) { - insertions += change.insertions; - deletions += change.deletions; - } + const stats = session + ? readSessionChangesStats(session, reader) ?? changesStatsCache.get(session.sessionId, reader) + : undefined; return { branch, - files: changes.length, - insertions, - deletions, + files: stats?.files ?? 0, + insertions: stats?.insertions ?? 0, + deletions: stats?.deletions ?? 0, } satisfies IDiffStats; }); @@ -351,6 +340,39 @@ class ViewAllChangesActionViewItemContribution extends Disposable implements IWo } } +/** + * Remembers the changes pill shown for each visible session so it can be rendered + * optimistically the next time that session is opened, before the provider has + * reported its changes. Recording sessions as they are shown (rather than from the + * pill itself) also keeps the cache honest: a session that ends up without changes + * drops its entry instead of keeping a stale pill. + */ +class SessionChangesStatsCacheContribution extends Disposable implements IWorkbenchContribution { + + static readonly ID = 'workbench.contrib.sessions.changesStatsCache'; + + constructor( + @ISessionsService sessionsService: ISessionsService, + @ISessionChangesStatsCache changesStatsCache: ISessionChangesStatsCache, + ) { + super(); + + this._register(autorun(reader => { + for (const session of sessionsService.visibleSessions.read(reader)) { + // While the worktree is pending the reported changes belong to the + // checkout the session was started from, not to the session. + if (!session || session.worktreePending?.read(reader)) { + continue; + } + const stats = readSessionChangesStats(session, reader); + if (stats) { + changesStatsCache.set(session.sessionId, stats); + } + } + })); + } +} + // --- Multi-diff source resolver /** @@ -469,3 +491,4 @@ class ChangesetOperationsActionControllerContribution extends Disposable impleme registerWorkbenchContribution2(ChangesMultiDiffSourceResolverContribution.ID, ChangesMultiDiffSourceResolverContribution, WorkbenchPhase.BlockRestore); registerWorkbenchContribution2(ChangesetOperationsActionControllerContribution.ID, ChangesetOperationsActionControllerContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(ViewAllChangesActionViewItemContribution.ID, ViewAllChangesActionViewItemContribution, WorkbenchPhase.AfterRestored); +registerWorkbenchContribution2(SessionChangesStatsCacheContribution.ID, SessionChangesStatsCacheContribution, WorkbenchPhase.AfterRestored); diff --git a/src/vs/sessions/contrib/chat/browser/newChatInput.ts b/src/vs/sessions/contrib/chat/browser/newChatInput.ts index 514664ea27a..ff6c110034c 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatInput.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatInput.ts @@ -799,13 +799,18 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation })); const dictationFocusKey = SessionsChatInputHasDictationFocus.bindTo(inputScopedContextKeyService); + // The composer is a chat input, so it carries the shared focus key that + // chat input keybindings such as paste as text are scoped to. + const inputHasFocusKey = ChatContextKeys.inputHasFocus.bindTo(inputScopedContextKeyService); this._register(this._editor.onDidFocusEditorWidget(() => { dictationFocusKey.set(true); + inputHasFocusKey.set(true); activeDictationComposer = this; this._onDidFocus.fire(); })); this._register(this._editor.onDidBlurEditorWidget(() => { dictationFocusKey.set(false); + inputHasFocusKey.set(false); if (activeDictationComposer === this) { activeDictationComposer = undefined; } diff --git a/src/vs/sessions/contrib/chat/browser/sessionArtifacts.ts b/src/vs/sessions/contrib/chat/browser/sessionArtifacts.ts index 4e9d089a8f5..594b46ef69d 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionArtifacts.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionArtifacts.ts @@ -6,20 +6,28 @@ import { Codicon } from '../../../../base/common/codicons.js'; import { MarkdownString } from '../../../../base/common/htmlContent.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; +import { getMediaMime } from '../../../../base/common/mime.js'; import { derived, IObservable, IReader } from '../../../../base/common/observable.js'; import { basename, getComparisonKey } from '../../../../base/common/resources.js'; import { ThemeIcon } from '../../../../base/common/themables.js'; import { URI } from '../../../../base/common/uri.js'; +import { generateUuid } from '../../../../base/common/uuid.js'; import { localize } from '../../../../nls.js'; import { toAction } from '../../../../base/common/actions.js'; import { IClipboardService } from '../../../../platform/clipboard/common/clipboardService.js'; +import { ICommandService } from '../../../../platform/commands/common/commands.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { observableConfigValue } from '../../../../platform/observable/common/platformObservableUtils.js'; import { IOpenerService } from '../../../../platform/opener/common/opener.js'; import type { IChatPillEntry, IChatPillSection } from '../../../../workbench/browser/chatPills.js'; import { openChatTurnFile, previewKind } from '../../../../workbench/contrib/chat/browser/widget/chatTurnPills.js'; +import { ChatConfiguration } from '../../../../workbench/contrib/chat/common/constants.js'; +import type { IImageCarouselCollection } from '../../../../workbench/contrib/imageCarousel/browser/imageCarouselTypes.js'; import { SessionArtifactKind, SessionFileOperation, type ISessionArtifact, type ISessionFile } from '../../../services/sessions/common/session.js'; import type { IActiveSession } from '../../../services/sessions/common/sessionsManagement.js'; +const OPEN_IMAGE_CAROUSEL_COMMAND_ID = 'workbench.action.chat.openImageInCarousel'; + const artifactIcons: ReadonlyMap = new Map([ [SessionArtifactKind.PullRequest, Codicon.gitPullRequest], [SessionArtifactKind.Issue, Codicon.issues], @@ -42,9 +50,15 @@ const sectionOrder: readonly { readonly kind: SessionArtifactKind; readonly titl export interface ISessionArtifactActions { openExternal(link: URI): void; openResource(uri: URI): void; + openImages(images: readonly ISessionArtifactImage[], startIndex: number): void; copy(text: string): void; } +export interface ISessionArtifactImage { + readonly uri: URI; + readonly mimeType: string; +} + function artifactValueKey(artifact: ISessionArtifact): string { if (artifact.uri) { return getComparisonKey(artifact.uri); @@ -52,7 +66,12 @@ function artifactValueKey(artifact: ISessionArtifact): string { return (artifact.link?.toString() ?? artifact.commitHash ?? artifact.id).toLowerCase(); } -function artifactLocation(uri: URI, label: string): Pick { +/** + * The location details shown for an artifact: its URI/link as the hover beside + * the dropdown row, the plain-text screen reader description, and the tooltip, + * while the accessible name stays the action the entry performs. + */ +export function sessionArtifactLocation(uri: URI, label: string): Pick { const value = uri.toString(true); return { ariaDescription: value, @@ -62,6 +81,11 @@ function artifactLocation(uri: URI, label: string): Pick actions.openResource(uri) }; + return { id: artifact.id, label, resource: uri, ...sessionArtifactLocation(uri, label), open: () => actions.openResource(uri) }; } const icon = artifactIcons.get(artifact.kind) ?? Codicon.archive; @@ -86,7 +110,7 @@ function toEntry(artifact: ISessionArtifact, actions: ISessionArtifactActions): run: () => actions.copy(artifact.commitHash!), })] : []; - return { id: artifact.id, label: artifact.label, icon, toolbarActions: copyAction, ...artifactLocation(link, artifact.label), open: () => actions.openExternal(link) }; + return { id: artifact.id, label: artifact.label, icon, toolbarActions: copyAction, ...sessionArtifactLocation(link, artifact.label), open: () => actions.openExternal(link) }; } if (artifact.kind === SessionArtifactKind.Resource) { @@ -94,14 +118,14 @@ function toEntry(artifact: ISessionArtifact, actions: ISessionArtifactActions): return undefined; } const uri = artifact.uri; - return { id: artifact.id, label: artifact.label, icon, ...artifactLocation(uri, artifact.label), open: () => actions.openResource(uri) }; + return { id: artifact.id, label: artifact.label, icon, ...sessionArtifactLocation(uri, artifact.label), open: () => actions.openResource(uri) }; } if (!artifact.link) { return undefined; } const link = artifact.link; - return { id: artifact.id, label: artifact.label, icon, ...artifactLocation(link, artifact.label), open: () => actions.openExternal(link) }; + return { id: artifact.id, label: artifact.label, icon, ...sessionArtifactLocation(link, artifact.label), open: () => actions.openExternal(link) }; } /** @@ -109,11 +133,20 @@ function toEntry(artifact: ISessionArtifact, actions: ISessionArtifactActions): * the previewable files the session wrote outside its workspace, de-duplicated * with the agent's own entries winning. */ -export function buildSessionArtifactSections(artifacts: readonly ISessionArtifact[], externalFiles: readonly ISessionFile[], actions: ISessionArtifactActions): readonly IChatPillSection[] { +export function buildSessionArtifactSections(artifacts: readonly ISessionArtifact[], externalFiles: readonly ISessionFile[], actions: ISessionArtifactActions, imageCarouselEnabled: boolean): readonly IChatPillSection[] { const entriesByKind = new Map(); + const images: ISessionArtifactImage[] = []; const seen = new Set(); for (const artifact of artifacts) { + const imageMimeType = artifact.uri ? getImageMimeType(artifact.uri) : undefined; + if (artifact.kind === SessionArtifactKind.File && artifact.uri && imageMimeType) { + if (!seen.has(artifactValueKey(artifact))) { + seen.add(artifactValueKey(artifact)); + images.push({ uri: artifact.uri, mimeType: imageMimeType }); + } + continue; + } const entry = toEntry(artifact, actions); if (!entry || seen.has(artifactValueKey(artifact))) { continue; @@ -125,18 +158,43 @@ export function buildSessionArtifactSections(artifacts: readonly ISessionArtifac } for (const file of externalFiles) { - if (file.operation === SessionFileOperation.Deleted || !previewKind(file.uri) || seen.has(getComparisonKey(file.uri))) { + const imageMimeType = getImageMimeType(file.uri); + if (file.operation === SessionFileOperation.Deleted || (!previewKind(file.uri) && !imageMimeType) || seen.has(getComparisonKey(file.uri))) { continue; } seen.add(getComparisonKey(file.uri)); + if (imageMimeType) { + images.push({ uri: file.uri, mimeType: imageMimeType }); + continue; + } const entries = entriesByKind.get(SessionArtifactKind.File) ?? []; const label = basename(file.uri); - entries.push({ id: file.uri.toString(), label, resource: file.uri, ...artifactLocation(file.uri, label), open: () => actions.openResource(file.uri) }); + entries.push({ id: file.uri.toString(), label, resource: file.uri, ...sessionArtifactLocation(file.uri, label), open: () => actions.openResource(file.uri) }); entriesByKind.set(SessionArtifactKind.File, entries); } const sections: IChatPillSection[] = []; for (const { kind, title } of sectionOrder) { + if (kind === SessionArtifactKind.File && images.length) { + sections.push({ + title: localize('sessionArtifacts.images', "Images"), + entries: images.map(({ uri }, index) => { + const label = basename(uri); + return { + id: uri.toString(), + label, + resource: uri, + ...sessionArtifactLocation(uri, label), + ...(imageCarouselEnabled + ? { + ariaLabel: localize('sessionArtifacts.openImage', "Open {0} in Images Preview", label), + open: () => actions.openImages(images, index), + } + : { open: () => actions.openResource(uri) }), + }; + }), + }); + } const entries = entriesByKind.get(kind); if (entries?.length) { sections.push({ title, entries }); @@ -153,11 +211,14 @@ export class SessionArtifacts extends Disposable { constructor( session: IObservable, @IClipboardService private readonly _clipboardService: IClipboardService, + @ICommandService private readonly _commandService: ICommandService, @IConfigurationService private readonly _configurationService: IConfigurationService, @IOpenerService private readonly _openerService: IOpenerService, ) { super(); + const imageCarouselEnabled = observableConfigValue(ChatConfiguration.ImageCarouselEnabled, true, this._configurationService); + this.sections = derived(this, reader => { const current = session.read(reader); if (!current) { @@ -167,6 +228,7 @@ export class SessionArtifacts extends Disposable { current.artifacts?.read(reader) ?? [], this._readExternalFiles(current, reader), this._actions(), + imageCarouselEnabled.read(reader), ); }); } @@ -185,6 +247,22 @@ export class SessionArtifacts extends Disposable { } void this._openerService.open(uri, { fromUserGesture: true }); }, + openImages: (images, startIndex) => { + const collection: IImageCarouselCollection = { + id: generateUuid(), + title: localize('sessionArtifacts.imageCarouselTitle', "Artifact Images"), + sections: [{ + title: localize('sessionArtifacts.images', "Images"), + images: images.map(image => ({ + id: image.uri.toString(), + name: basename(image.uri), + mimeType: image.mimeType, + uri: image.uri, + })), + }], + }; + void this._commandService.executeCommand(OPEN_IMAGE_CAROUSEL_COMMAND_ID, { collection, startIndex }); + }, copy: text => { void this._clipboardService.writeText(text); }, }; } diff --git a/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts b/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts index 23f98e8fb7c..dce749fb093 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts @@ -7,7 +7,6 @@ import { $, addDisposableListener, DisposableResizeObserver, EventType, getWindo import { StandardMouseEvent } from '../../../../base/browser/mouseEvent.js'; import { DomScrollableElement } from '../../../../base/browser/ui/scrollbar/scrollableElement.js'; import { toAction, Action, Separator, type IAction } from '../../../../base/common/actions.js'; -import { MarkdownString } from '../../../../base/common/htmlContent.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; import { autorun, derived, derivedOpts, IObservable, IReader, observableValue } from '../../../../base/common/observable.js'; import { isEqual } from '../../../../base/common/resources.js'; @@ -18,7 +17,7 @@ import { IContextMenuService } from '../../../../platform/contextview/browser/co import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; import { IChatResponseFileChangesService } from '../../../../workbench/contrib/chat/browser/chatResponseFileChangesService.js'; import { CHAT_TURN_ARTIFACT_PILL_ID, CHAT_TURN_CHANGES_PILL_ID, ChatTurnPillsProvider, diffStatsEqual, EMPTY_DIFF_STATS, IChatTurnPillsModel, IDiffStats, observeTurnStatusPillsEnabled } from '../../../../workbench/contrib/chat/browser/widget/chatTurnPills.js'; -import { SessionArtifacts } from './sessionArtifacts.js'; +import { SessionArtifacts, sessionArtifactLocation } from './sessionArtifacts.js'; import { chatCustomizationPillOptions, SessionCustomizations, SESSION_CUSTOMIZATIONS_PILL_ID } from './sessionCustomizations.js'; import { localize } from '../../../../nls.js'; import { getChatPillEntries, ChatPillsWidget, IChatPill, IChatPillsModel, type IChatPillSection } from '../../../../workbench/browser/chatPills.js'; @@ -54,17 +53,7 @@ function computeTurnStats(chat: IChat, reader: IReader): IDiffStats { function buildDebugArtifactSections(debugData: ISessionChatPillsDebugData): readonly IChatPillSection[] { const entries = debugData.markdownFiles.map(name => { const resource = URI.from({ scheme: 'session-chat-pills-debug', path: `/${name}` }); - const location = resource.toString(true); - return { - id: name, - label: name, - resource, - ariaDescription: location, - ariaLabel: localize('sessionArtifacts.open', "Open {0}", name), - hover: { content: new MarkdownString().appendText(location) }, - tooltip: location, - open: () => { }, - }; + return { id: name, label: name, resource, ...sessionArtifactLocation(resource, name), open: () => { } }; }); return entries.length ? [{ title: localize('sessionArtifacts.files', "Files"), entries }] : []; } @@ -240,7 +229,7 @@ export class SessionChatInputToolbar extends Disposable { pills.element.classList.add('show-file-icons'); this._content.appendChild(pills.element); - // Kinds the session reports data for; the others cannot be toggled. + // Kinds the session reports data for; the others are listed in a separate group. const kindsWithData = derived(reader => { const kinds = new Set(); for (const pill of candidatePills.read(reader)) { @@ -281,7 +270,6 @@ export class SessionChatInputToolbar extends Disposable { id: `sessions.chatPills.toggle.${entry.kind}`, label: entry.label, checked: entry.checked, - enabled: entry.enabled, run: () => visibility.toggle(entry.kind), }); diff --git a/src/vs/sessions/contrib/chat/browser/sessionMetadataPills.ts b/src/vs/sessions/contrib/chat/browser/sessionMetadataPills.ts index 4c160b05c84..a86d658fcce 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionMetadataPills.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionMetadataPills.ts @@ -18,6 +18,7 @@ import { Menus } from '../../../browser/menus.js'; import { ISessionContext, SessionContext } from '../../../services/sessions/browser/sessionContext.js'; import { IActiveSession } from '../../../services/sessions/common/sessionsManagement.js'; import { setSessionContextKeys } from '../../../services/sessions/common/sessionContextKeys.js'; +import { ISessionChangesStatsCache } from '../../../services/sessions/common/sessionChangesStatsCache.js'; /** Adapts the session metadata menu to observable chat-pill descriptors. */ export class SessionMetadataPills extends Disposable { @@ -33,6 +34,7 @@ export class SessionMetadataPills extends Disposable { @IContextKeyService contextKeyService: IContextKeyService, @IInstantiationService instantiationService: IInstantiationService, @IMenuService menuService: IMenuService, + @ISessionChangesStatsCache changesStatsCache: ISessionChangesStatsCache, ) { super(); @@ -43,7 +45,7 @@ export class SessionMetadataPills extends Disposable { ))); this._register(autorun(reader => { - setSessionContextKeys(session.read(reader), scopedContextKeyService, reader); + setSessionContextKeys(session.read(reader), scopedContextKeyService, reader, changesStatsCache); })); const menu = this._register(menuService.createMenu(Menus.SessionHeaderMeta, scopedContextKeyService, { emitEventsForSubmenuChanges: true })); diff --git a/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts b/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts index 5f45f7fd63f..327c7648469 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts @@ -56,6 +56,7 @@ export class SessionsChatAccessibilityHelp implements IAccessibleViewImplementat content.push(localize('sessionsChat.micContextMenu', "To choose a microphone or turn off dictation or Voice Mode, focus the microphone button in the input toolbar and open its context menu (for example Shift+F10).")); content.push(localize('sessionsChat.contextReferences', "Type # in the chat input to attach context. Use #file to reference a file or folder, or #session to reference another agent session. Referencing a session together with the /troubleshoot command analyzes that session's logs instead of the current one. Accept a suggestion with Tab or Enter; the reference appears as a pill above the input that you can remove.")); content.push(localize('sessionsChat.pastedText', "Long pasted text is stored as an attached text item and replaced in the input with a numbered inline reference.")); + content.push(localize('sessionsChat.pasteAsText', "To paste the clipboard as plain text, without converting it to Markdown or storing it as an attachment, invoke Paste as Text{0}.", '')); content.push(localize('sessionsChat.backgroundActivities', "Press Shift+Tab from the chat input to reach metadata and status pills above it, then press Enter or Space to activate a pill. Live browsers appear in their own pill, and background activities such as running subagents in another. A pill with more than one entry opens a picker; use the up and down arrows to navigate, Enter to open an entry, and Escape to dismiss the picker and return focus to the pill.")); content.push(localize('sessionsChat.conversations', "When multiple chats appear as tabs in a single group, the tab row replaces the session header and includes the session actions. Side-by-side chat groups retain the session header and keep their tab rows compact. Activate New Chat at the end of a tab row to start another chat in that group.")); content.push(localize('sessionsChat.subagentPills', "Subagent pills in the chat transcript can be dragged to a chat group's edge to open the subagent beside the current chat. With the keyboard, focus a subagent pill and press Alt+Enter to open it beside the current chat.")); diff --git a/src/vs/sessions/contrib/chat/common/sessionChatPills.ts b/src/vs/sessions/contrib/chat/common/sessionChatPills.ts index d44449367ad..188d8054a80 100644 --- a/src/vs/sessions/contrib/chat/common/sessionChatPills.ts +++ b/src/vs/sessions/contrib/chat/common/sessionChatPills.ts @@ -57,8 +57,6 @@ export interface ISessionChatPillMenuEntry { readonly label: string; /** Whether the pill shows when it has data. */ readonly checked: boolean; - /** Kinds without data cannot be toggled. */ - readonly enabled: boolean; } /** @@ -73,8 +71,8 @@ export interface ISessionChatPillMenu { } /** - * Builds the visibility menu. Every hideable kind is listed, checked while it is - * not hidden, and disabled while the session reports no data for it. + * Builds the visibility menu. Every hideable kind is listed and toggleable, + * checked while it is not hidden, grouped by whether the session has data for it. * * @param targetKind The pill that was right-clicked, which gains a "Hide X" * entry. Omitted when the click did not land on a pill. @@ -90,12 +88,10 @@ export function getSessionChatPillMenu( if (!isSessionChatPillHideable(kind)) { continue; } - const enabled = kindsWithData.has(kind); - (enabled ? withData : withoutData).push({ + (kindsWithData.has(kind) ? withData : withoutData).push({ kind, label: getSessionChatPillLabel(kind), checked: !hiddenKinds.has(kind), - enabled, }); } diff --git a/src/vs/sessions/contrib/chat/test/browser/newChatInputPaste.test.ts b/src/vs/sessions/contrib/chat/test/browser/newChatInputPaste.test.ts index 0e9d5e08d0d..cb91a2f5091 100644 --- a/src/vs/sessions/contrib/chat/test/browser/newChatInputPaste.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/newChatInputPaste.test.ts @@ -21,8 +21,9 @@ import { createTextModel } from '../../../../../editor/test/common/testTextModel import { withTestCodeEditor } from '../../../../../editor/test/browser/testCodeEditor.js'; import { ServiceCollection } from '../../../../../platform/instantiation/common/serviceCollection.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { IChatPasteTarget, IChatPasteTargetService } from '../../../../../workbench/contrib/chat/browser/chat.js'; -import { PasteTextProvider } from '../../../../../workbench/contrib/chat/browser/widget/input/editor/chatPasteProviders.js'; +import { PasteTextProvider, pastedTextArtifactDefaultMinLength } from '../../../../../workbench/contrib/chat/browser/widget/input/editor/chatPasteProviders.js'; import { IChatRequestVariableEntry, isPastedTextArtifact } from '../../../../../workbench/contrib/chat/common/attachments/chatVariableEntries.js'; import { IChatSessionsService } from '../../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { IActiveSession } from '../../../../services/sessions/common/sessionsManagement.js'; @@ -113,6 +114,9 @@ suite('NewChatInputPasteTarget', () => { pasteTargetService, new class extends mock() { }, new class extends mock() { }, + new class extends mock() { + override getValue(): T { return pastedTextArtifactDefaultMinLength as T; } + }, ); const transfer = new VSDataTransfer(); @@ -167,7 +171,7 @@ suite('NewChatInputPasteTarget', () => { } test('keeps the attachment and its inline reference consistent across undo and redo', async () => { - const pastedText = 'x'.repeat(1200); + const pastedText = `${'x'.repeat(1200)}\n`.repeat(10); const snapshots = await runPasteLifecycle(pastedText); const attached = { attachments: ['Pasted text #1'], codeIsPreserved: true, sent: [{ name: 'Pasted text #1', text: '#attachment:Pasted text #1' }] }; @@ -188,7 +192,7 @@ suite('NewChatInputPasteTarget', () => { }); test('removing the attachment takes its inline reference out of the input', async () => { - const pastedText = 'x'.repeat(1200); + const pastedText = `${'x'.repeat(1200)}\n`.repeat(10); const snapshots = await runPasteLifecycle(pastedText, attachments => { attachments.removeAttachment(attachments.attachments[0].id); }); diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionArtifacts.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionArtifacts.test.ts index cdda336a07b..0472982acac 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionArtifacts.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionArtifacts.test.ts @@ -7,7 +7,7 @@ import assert from 'assert'; import { isMarkdownString } from '../../../../../base/common/htmlContent.js'; import { URI } from '../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; -import { buildSessionArtifactSections, type ISessionArtifactActions } from '../../browser/sessionArtifacts.js'; +import { buildSessionArtifactSections, type ISessionArtifactActions, type ISessionArtifactImage } from '../../browser/sessionArtifacts.js'; import { type ISessionArtifact, SessionArtifactKind, SessionFileOperation } from '../../../../services/sessions/common/session.js'; suite('Session Artifacts', () => { @@ -16,6 +16,7 @@ suite('Session Artifacts', () => { const actions: ISessionArtifactActions = { openExternal() { }, openResource() { }, + openImages() { }, copy() { }, }; @@ -30,7 +31,7 @@ suite('Session Artifacts', () => { { id: 'resource', kind: SessionArtifactKind.Resource, label: 'Resource', uri: resourceUri }, ]; - const entries = buildSessionArtifactSections(artifacts, [{ uri: externalFileUri, operation: SessionFileOperation.Created }], actions).flatMap(section => section.entries); + const entries = buildSessionArtifactSections(artifacts, [{ uri: externalFileUri, operation: SessionFileOperation.Created }], actions, true).flatMap(section => section.entries); assert.deepStrictEqual(entries.map(entry => { const content = entry.hover?.content; return { @@ -47,4 +48,63 @@ suite('Session Artifacts', () => { { label: 'Resource', ariaLabel: 'Open Resource', ariaDescription: resourceUri.toString(true), hover: resourceUri.toString(true), tooltip: resourceUri.toString(true) }, ]); }); + + test('groups artifact images separately and opens all images in the carousel', () => { + const screenshotUri = URI.file('/artifacts/screenshot.png'); + const diagramUri = URI.file('/external/diagram.jpg'); + const reportUri = URI.file('/artifacts/report.md'); + const opened: { images: readonly ISessionArtifactImage[]; startIndex: number }[] = []; + const imageActions: ISessionArtifactActions = { + ...actions, + openImages: (images, startIndex) => opened.push({ images, startIndex }), + }; + const artifacts: readonly ISessionArtifact[] = [ + { id: 'screenshot', kind: SessionArtifactKind.File, label: 'Screenshot', uri: screenshotUri }, + { id: 'report', kind: SessionArtifactKind.File, label: 'Report', uri: reportUri }, + ]; + + const sections = buildSessionArtifactSections(artifacts, [ + { uri: diagramUri, operation: SessionFileOperation.Created }, + ], imageActions, true); + const imageSection = sections.find(section => section.title === 'Images'); + assert.ok(imageSection); + imageSection.entries[1].open(); + + assert.deepStrictEqual({ + sections: sections.map(section => ({ title: section.title, labels: section.entries.map(entry => entry.label) })), + opened: opened.map(entry => ({ images: entry.images.map(image => image.uri.path), startIndex: entry.startIndex })), + }, { + sections: [ + { title: 'Images', labels: ['screenshot.png', 'diagram.jpg'] }, + { title: 'Files', labels: ['report.md'] }, + ], + opened: [{ images: ['/artifacts/screenshot.png', '/external/diagram.jpg'], startIndex: 1 }], + }); + }); + + test('opens the image resource when the image carousel is disabled', () => { + const screenshotUri = URI.file('/artifacts/screenshot.png'); + const opened: string[] = []; + const imageActions: ISessionArtifactActions = { + ...actions, + openImages: () => opened.push('carousel'), + openResource: uri => opened.push(uri.path), + }; + const artifacts: readonly ISessionArtifact[] = [ + { id: 'screenshot', kind: SessionArtifactKind.File, label: 'Screenshot', uri: screenshotUri }, + ]; + + const sections = buildSessionArtifactSections(artifacts, [], imageActions, false); + const imageSection = sections.find(section => section.title === 'Images'); + assert.ok(imageSection); + imageSection.entries[0].open(); + + assert.deepStrictEqual({ + ariaLabel: imageSection.entries[0].ariaLabel, + opened, + }, { + ariaLabel: 'Open screenshot.png', + opened: ['/artifacts/screenshot.png'], + }); + }); }); diff --git a/src/vs/sessions/contrib/chat/test/common/sessionChatPills.test.ts b/src/vs/sessions/contrib/chat/test/common/sessionChatPills.test.ts index f3d44b8f17b..a0e175a04e8 100644 --- a/src/vs/sessions/contrib/chat/test/common/sessionChatPills.test.ts +++ b/src/vs/sessions/contrib/chat/test/common/sessionChatPills.test.ts @@ -19,14 +19,14 @@ suite('SessionChatPills', () => { assert.deepStrictEqual(menu, { withData: [ - { kind: SessionChatPillKind.PullRequests, label: 'Pull Requests', checked: false, enabled: true }, - { kind: SessionChatPillKind.Subagents, label: 'Subagents', checked: true, enabled: true }, + { kind: SessionChatPillKind.PullRequests, label: 'Pull Requests', checked: false }, + { kind: SessionChatPillKind.Subagents, label: 'Subagents', checked: true }, ], withoutData: [ - { kind: SessionChatPillKind.Artifacts, label: 'Artifacts', checked: true, enabled: false }, - { kind: SessionChatPillKind.Customizations, label: 'Customizations', checked: true, enabled: false }, - { kind: SessionChatPillKind.Issues, label: 'Issues', checked: true, enabled: false }, - { kind: SessionChatPillKind.Browsers, label: 'Browsers', checked: true, enabled: false }, + { kind: SessionChatPillKind.Artifacts, label: 'Artifacts', checked: true }, + { kind: SessionChatPillKind.Customizations, label: 'Customizations', checked: true }, + { kind: SessionChatPillKind.Issues, label: 'Issues', checked: true }, + { kind: SessionChatPillKind.Browsers, label: 'Browsers', checked: true }, ], }); }); diff --git a/src/vs/sessions/contrib/github/browser/createSessionFromPullRequestAction.ts b/src/vs/sessions/contrib/github/browser/createSessionFromPullRequestAction.ts index c12765fbedb..b595f42bc3d 100644 --- a/src/vs/sessions/contrib/github/browser/createSessionFromPullRequestAction.ts +++ b/src/vs/sessions/contrib/github/browser/createSessionFromPullRequestAction.ts @@ -27,7 +27,7 @@ import { Menus } from '../../../browser/menus.js'; import { ISessionSection, SessionSectionHasGitHubRepositoryContext, SessionSectionHasNonCloudRepositoryContext, SessionSectionTypeContext } from '../../sessions/browser/views/sessionsList.js'; import { IGitHubService } from './githubService.js'; import { IGitHubPullRequestSummary } from '../common/types.js'; -import { createPullRequestBootstrapPrompt, createPullRequestContextAttachment, createPullRequestQuickPickItems, createPullRequestSessionMetadata, getExistingPullRequests, hasExistingPullRequest, IPullRequestQuickPickItem, mergePullRequestSummaries, pullRequestMatchesQuery, resolvePullRequestSessionRepository } from './pullRequestPicker.js'; +import { createPullRequestBootstrapPrompt, createPullRequestContextAttachment, createPullRequestQuickPickItems, createPullRequestSessionMetadata, getExistingPullRequests, IPullRequestQuickPickItem, isPullRequestAvailable, mergePullRequestSummaries, pullRequestMatchesQuery, resolvePullRequestSessionRepository } from './pullRequestPicker.js'; import { createAndOpenPullRequestSession } from './pullRequestSessionCreation.js'; export const NEW_SESSION_FROM_PULL_REQUEST_COMMAND_ID = 'workbench.agentSessions.newSessionFromPullRequest'; @@ -192,7 +192,7 @@ registerAction2(class NewSessionFromPullRequestAction extends Action2 { const initialGroupsPromise = loadInitialGroups(); const loadUntilMatch = async (query: string, generation: number): Promise => { await initialGroupsPromise; - while (generation === searchGeneration && query && hasNextPage && !pullRequests.some(pullRequest => !hasExistingPullRequest(pullRequest, existingPullRequests) && pullRequestMatchesQuery(pullRequest, query))) { + while (generation === searchGeneration && query && hasNextPage && !pullRequests.some(pullRequest => isPullRequestAvailable(pullRequest, existingPullRequests) && pullRequestMatchesQuery(pullRequest, query))) { await loadNextPage(); } }; @@ -231,8 +231,9 @@ registerAction2(class NewSessionFromPullRequestAction extends Action2 { }, }, { isolationMode: 'worktree', - branch: pullRequest.checkoutRef, + branch: pullRequest.headRef, worktreeBranchTrack: true, + worktreeCreateNewBranch: false, metadata: createPullRequestSessionMetadata(repository.owner, repository.repo, pullRequest), onSessionCreated, }, pickerCts.token), diff --git a/src/vs/sessions/contrib/github/browser/fetchers/githubPullRequestsFetcher.ts b/src/vs/sessions/contrib/github/browser/fetchers/githubPullRequestsFetcher.ts index 06890bbcc18..8db95cbe198 100644 --- a/src/vs/sessions/contrib/github/browser/fetchers/githubPullRequestsFetcher.ts +++ b/src/vs/sessions/contrib/github/browser/fetchers/githubPullRequestsFetcher.ts @@ -23,6 +23,7 @@ interface IGitHubPullRequestNode { readonly title: string; readonly author: { readonly login: string; readonly avatarUrl: string } | null; readonly headRefName: string; + readonly isCrossRepository: boolean; readonly isDraft: boolean; readonly updatedAt: string; readonly additions: number; @@ -42,6 +43,7 @@ const LIST_PULL_REQUESTS_QUERY = [ ' title', ' author { login avatarUrl }', ' headRefName', + ' isCrossRepository', ' isDraft', ' updatedAt', ' additions', @@ -62,6 +64,7 @@ const LIST_PULL_REQUEST_NUMBERS_QUERY = [ ' title', ' author { login avatarUrl }', ' headRefName', + ' isCrossRepository', ' isDraft', ' updatedAt', ' additions', @@ -123,6 +126,7 @@ function mapPullRequest(pullRequest: IGitHubPullRequestNode, reviewRequestedFrom author: pullRequest.author ?? { login: 'ghost', avatarUrl: '' }, headRef: pullRequest.headRefName, checkoutRef: `refs/pull/${pullRequest.number}/head`, + isCrossRepository: pullRequest.isCrossRepository, isDraft: pullRequest.isDraft, updatedAt: pullRequest.updatedAt, additions: pullRequest.additions, diff --git a/src/vs/sessions/contrib/github/browser/pullRequestPicker.ts b/src/vs/sessions/contrib/github/browser/pullRequestPicker.ts index 816309c36a0..0c8ef2f9ff4 100644 --- a/src/vs/sessions/contrib/github/browser/pullRequestPicker.ts +++ b/src/vs/sessions/contrib/github/browser/pullRequestPicker.ts @@ -111,8 +111,12 @@ export function hasExistingPullRequest(pullRequest: IGitHubPullRequestSummary, e return existingPullRequests.numbers.has(pullRequest.number) || existingPullRequests.headRefs.has(pullRequest.headRef); } +export function isPullRequestAvailable(pullRequest: IGitHubPullRequestSummary, existingPullRequests: IExistingPullRequests): boolean { + return !pullRequest.isCrossRepository && !hasExistingPullRequest(pullRequest, existingPullRequests); +} + export function createPullRequestQuickPickItems(pullRequests: readonly IGitHubPullRequestSummary[], existingPullRequests: IExistingPullRequests): readonly (IPullRequestQuickPickItem | IQuickPickSeparator)[] { - const available = pullRequests.filter(pullRequest => !hasExistingPullRequest(pullRequest, existingPullRequests)); + const available = pullRequests.filter(pullRequest => isPullRequestAvailable(pullRequest, existingPullRequests)); const waitingForReview = available.filter(pullRequest => pullRequest.reviewRequestedFromViewer); const assigned = available.filter(pullRequest => !pullRequest.reviewRequestedFromViewer && pullRequest.assignedToViewer); const other = available.filter(pullRequest => !pullRequest.reviewRequestedFromViewer && !pullRequest.assignedToViewer); diff --git a/src/vs/sessions/contrib/github/common/types.ts b/src/vs/sessions/contrib/github/common/types.ts index 4eb65c42589..49450508949 100644 --- a/src/vs/sessions/contrib/github/common/types.ts +++ b/src/vs/sessions/contrib/github/common/types.ts @@ -80,6 +80,7 @@ export interface IGitHubPullRequestSummary { readonly author: IGitHubUser; readonly headRef: string; readonly checkoutRef: string; + readonly isCrossRepository: boolean; readonly isDraft: boolean; readonly updatedAt: string; readonly additions: number; diff --git a/src/vs/sessions/contrib/github/test/browser/githubFetchers.test.ts b/src/vs/sessions/contrib/github/test/browser/githubFetchers.test.ts index 288140f26e7..dabdf8f3d7d 100644 --- a/src/vs/sessions/contrib/github/test/browser/githubFetchers.test.ts +++ b/src/vs/sessions/contrib/github/test/browser/githubFetchers.test.ts @@ -450,6 +450,7 @@ suite('GitHubPullRequestsFetcher', () => { title: 'Improve sessions', author: { login: 'author', avatarUrl: 'avatar' }, headRefName: 'feature', + isCrossRepository: false, isDraft: true, updatedAt: '2026-07-30T12:00:00Z', additions: 12, @@ -470,6 +471,7 @@ suite('GitHubPullRequestsFetcher', () => { author: { login: 'author', avatarUrl: 'avatar' }, headRef: 'feature', checkoutRef: 'refs/pull/7/head', + isCrossRepository: false, isDraft: true, updatedAt: '2026-07-30T12:00:00Z', additions: 12, @@ -657,6 +659,7 @@ function makePullRequestSearchNode(number: number): unknown { title: `Pull request ${number}`, author: { login: 'author', avatarUrl: '' }, headRefName: `feature-${number}`, + isCrossRepository: false, isDraft: false, updatedAt: '2026-07-30T12:00:00Z', additions: number, diff --git a/src/vs/sessions/contrib/github/test/browser/pullRequestPicker.test.ts b/src/vs/sessions/contrib/github/test/browser/pullRequestPicker.test.ts index eaef8075e93..a50e5c72305 100644 --- a/src/vs/sessions/contrib/github/test/browser/pullRequestPicker.test.ts +++ b/src/vs/sessions/contrib/github/test/browser/pullRequestPicker.test.ts @@ -13,7 +13,7 @@ import { mock } from '../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { readSessionGitHubState } from '../../../../../platform/agentHost/common/state/sessionState.js'; import { ISession, ISessionWorkspace } from '../../../../services/sessions/common/session.js'; -import { createPullRequestBootstrapPrompt, createPullRequestContextAttachment, createPullRequestQuickPickItems, createPullRequestSessionMetadata, getExistingPullRequests, getPullRequestNumberFromCheckoutRef, IPullRequestQuickPickItem, mergePullRequestSummaries, pullRequestMatchesQuery, resolvePullRequestSessionRepository } from '../../browser/pullRequestPicker.js'; +import { createPullRequestBootstrapPrompt, createPullRequestContextAttachment, createPullRequestQuickPickItems, createPullRequestSessionMetadata, getExistingPullRequests, getPullRequestNumberFromCheckoutRef, IPullRequestQuickPickItem, isPullRequestAvailable, mergePullRequestSummaries, pullRequestMatchesQuery, resolvePullRequestSessionRepository } from '../../browser/pullRequestPicker.js'; import { IGitHubPullRequestSummary } from '../../common/types.js'; import { createAndOpenPullRequestSession } from '../../browser/pullRequestSessionCreation.js'; @@ -86,6 +86,22 @@ suite('Create Session from Pull Request', () => { }); }); + test('only makes same-repository pull requests without existing sessions available', () => { + const existingPullRequests = { numbers: new Set([1]), headRefs: new Set(['feature-2']) }; + + assert.deepStrictEqual([ + pullRequest(1), + pullRequest(2), + pullRequest(3, { isCrossRepository: true }), + pullRequest(4), + ].map(item => isPullRequestAvailable(item, existingPullRequests)), [ + false, + false, + false, + true, + ]); + }); + test('merges viewer-group results into the loaded catalog without dropping either set', () => { const merged = mergePullRequestSummaries([ pullRequest(1), @@ -295,6 +311,7 @@ function pullRequest(number: number, overrides: Partial