diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts index 8f350559bda02..8feea1a6cf0b1 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts @@ -35,7 +35,7 @@ import { SingleSlotTtlCache, TtlCache } from '../common/ttlCache'; import { isUntitledSessionId } from '../common/utils'; import { IChatDelegationSummaryService } from '../copilotcli/common/delegationSummaryService'; import { CONTINUE_TRUNCATION, extractTitle, getAuthorDisplayName, getRepoId, isActiveTaskState, SessionIdForPr, SessionIdForTask, toOpenPullRequestWebviewUri, truncatePrompt } from '../vscode/copilotCodingAgentUtils'; -import { CloudAgentBackend, PullArtifactRef, TaskCloudAgentBackend, TaskContent } from '../vscode/cloudAgentBackend'; +import { CloudAgentBackend, CloudSessionData, PullArtifactRef, TaskCloudAgentBackend, TaskContent } from '../vscode/cloudAgentBackend'; import { CopilotCloudGitOperationsManager } from './copilotCloudGitOperationsManager'; import { ChatSessionContentBuilder, SessionResponseLogChunk } from './copilotCloudSessionContentBuilder'; import { StreamBaseline, TaskTurnStreamer } from './taskTurnStreamer'; @@ -141,6 +141,18 @@ export function normalizeInitialSessionOptions(initialOptions: unknown, logServi return []; } +export function getCloudSessionItemMetadata(repo: CloudSessionData['repo'], diffRefs: CloudSessionData['diffRefs']): { readonly owner: string; readonly name: string; readonly host?: string; readonly branch?: string } | undefined { + if (!repo) { + return undefined; + } + return { + owner: repo.owner, + name: repo.name, + ...(repo.host ? { host: repo.host } : {}), + ...(diffRefs?.headRef ? { branch: diffRefs.headRef } : {}), + }; +} + export function parseSessionLogChunksSafely(rawText: string, logService: ILogService, parser: (value: string) => SessionResponseLogChunk[]): SessionResponseLogChunk[] { try { return parser(rawText); @@ -1327,12 +1339,13 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C const changes = entry.diffRefs ? await this._prFileChangesService.getComparisonChangedFiles(entry.diffRefs) : undefined; + const metadata = getCloudSessionItemMetadata(entry.repo, entry.diffRefs); return { resource: vscode.Uri.from({ scheme: CopilotCloudSessionsProvider.TYPE, path: '/' + SessionIdForTask.getId(taskId) }), label: entry.latestSession.name || taskId, status, ...(changes?.length ? { changes } : {}), - ...(entry.repo ? { metadata: { owner: entry.repo.owner, name: entry.repo.name } } : {}), + ...(metadata ? { metadata } : {}), ...(createdAt ? { timing: { created: createdAt, diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/taskApiBackend.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/taskApiBackend.ts index e9d9483ed2c5d..cb19abd8c4105 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/taskApiBackend.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/taskApiBackend.ts @@ -145,15 +145,15 @@ function taskToSessionInfo(task: AgentTask): SessionInfo { * Also used by the provider to derive `{owner, repo}` for the "Create pull request" toolbar * action on PR-less tasks. */ -export function parseRepoFromTaskUrl(htmlUrl: string | undefined): { owner: string; name: string } | undefined { +export function parseRepoFromTaskUrl(htmlUrl: string | undefined): { owner: string; name: string; host: string } | undefined { if (!htmlUrl) { return undefined; } try { - const { pathname } = new URL(htmlUrl); + const { hostname, pathname } = new URL(htmlUrl); const match = pathname.match(/^\/([^/]+)\/([^/]+)\//); if (match) { - return { owner: match[1], name: match[2] }; + return { owner: match[1], name: match[2], host: hostname }; } } catch { // not a parseable URL @@ -281,7 +281,7 @@ export class TaskApiBackend implements TaskCloudAgentBackend { async fetchSessionList(repoIds: GithubRepoId[] | undefined, isAgentWorkspace: boolean, _refresh: boolean): Promise { const listOpts: ListTasksOptions = { per_page: 100 }; - const tasksWithRepo: { task: AgentTask; repo: { owner: string; name: string } | undefined }[] = []; + const tasksWithRepo: { task: AgentTask; repo: CloudSessionData['repo'] }[] = []; // In the agents window we surface all of the user's sessions rather than scoping to the // active workspace's repositories, so always use the global user-scoped list there. @@ -308,11 +308,11 @@ export class TaskApiBackend implements TaskCloudAgentBackend { repoIds.map(async repo => { try { const r = await this._taskApiClient.listTasksForRepo(repo.org, repo.repo, repoListOpts); - return { repo: { owner: repo.org, name: repo.repo }, response: r }; + return { repo: { owner: repo.org, name: repo.repo, host: repo.host }, response: r }; } catch (e: unknown) { this._logService.warn(`Failed to fetch tasks for ${repo.org}/${repo.repo}: ${e}`); this._instrumentation.operationFailed('fetchSessionList', e); - return { repo: { owner: repo.org, name: repo.repo }, response: { tasks: [] as readonly AgentTask[] } satisfies AgentTaskListResponse }; + return { repo: { owner: repo.org, name: repo.repo, host: repo.host }, response: { tasks: [] as readonly AgentTask[] } satisfies AgentTaskListResponse }; } }), ); @@ -329,7 +329,7 @@ export class TaskApiBackend implements TaskCloudAgentBackend { // id lookup is cached (by promise) per repo id so a page of same-repo tasks — resolved // concurrently via `Promise.all` — shares a single in-flight call. const repoByIdCache = new Map>(); - const resolveRepo = (task: AgentTask, listRepo: { owner: string; name: string } | undefined): Promise<{ owner: string; name: string } | undefined> => { + const resolveRepo = (task: AgentTask, listRepo: CloudSessionData['repo']): Promise => { const known = listRepo ?? parseRepoFromTaskUrl(task.html_url); if (known) { return Promise.resolve(known); diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCloudSessionsProvider.spec.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCloudSessionsProvider.spec.ts index 89c6fb7388f58..f903257556a01 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCloudSessionsProvider.spec.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCloudSessionsProvider.spec.ts @@ -13,7 +13,7 @@ import { mock } from '../../../../util/common/test/simpleMock'; import { ChatRequestTurn2, ChatResponseMarkdownPart, ChatResponseTurn2, ChatToolInvocationPart } from '../../../../vscodeTypes'; import { ITaskApiClient, ListTaskEventsOptions, ListTasksOptions } from '../../common/taskApiTypes'; import { ChatSessionContentBuilder, extractTaskErrorDetail, formatTaskStoppedMessage } from '../copilotCloudSessionContentBuilder'; -import { normalizeInitialSessionOptions, parseSessionLogChunksSafely, taskStateToChatSessionStatus } from '../copilotCloudSessionsProvider'; +import { getCloudSessionItemMetadata, normalizeInitialSessionOptions, parseSessionLogChunksSafely, taskStateToChatSessionStatus } from '../copilotCloudSessionsProvider'; import { TaskApiBackend, parseRepoFromTaskUrl, isCloudCodingAgentTask } from '../taskApiBackend'; import { isActiveTaskState, isFailedTaskState } from '../../vscode/copilotCodingAgentUtils'; import { NullCloudBackendInstrumentation } from '../cloudBackendTelemetry'; @@ -129,6 +129,18 @@ describe('copilotCloudSessionsProvider helpers', () => { expect(result).toEqual([]); expect(logService.error).toHaveBeenCalledWith(expect.any(SyntaxError), expect.stringContaining('Failed to parse streamed log content')); }); + + it('includes the task branch in PR-less cloud session metadata', () => { + expect(getCloudSessionItemMetadata( + { owner: 'microsoft', name: 'vscode', host: 'github.com' }, + { owner: 'microsoft', repo: 'vscode', baseRef: 'main', headRef: 'copilot/task-branch' }, + )).toEqual({ + owner: 'microsoft', + name: 'vscode', + host: 'github.com', + branch: 'copilot/task-branch', + }); + }); }); describe('ChatSessionContentBuilder', () => { @@ -720,7 +732,7 @@ describe('isActiveTaskState / isFailedTaskState', () => { describe('parseRepoFromTaskUrl', () => { it('extracts owner and name from a task html_url', () => { - expect(parseRepoFromTaskUrl('https://github.com/octocat/hello-world/agents/tasks/abc')).toEqual({ owner: 'octocat', name: 'hello-world' }); + expect(parseRepoFromTaskUrl('https://github.example.com/octocat/hello-world/agents/tasks/abc')).toEqual({ owner: 'octocat', name: 'hello-world', host: 'github.example.com' }); }); it('returns undefined for an unparseable URL', () => { diff --git a/extensions/copilot/src/extension/chatSessions/vscode/cloudAgentBackend.ts b/extensions/copilot/src/extension/chatSessions/vscode/cloudAgentBackend.ts index ef2d80ca561bd..54e5259d66bcd 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode/cloudAgentBackend.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode/cloudAgentBackend.ts @@ -89,7 +89,7 @@ export interface CloudSessionData { * attach `owner`/`name` metadata to PR-less task cards (otherwise they group under * "Unknown"/"Other" until a PR resolves) and passes it to `resolvePullArtifact` for PR lookup. */ - readonly repo?: { readonly owner: string; readonly name: string }; + readonly repo?: { readonly owner: string; readonly name: string; readonly host?: string }; /** * Branch comparison refs for a settled, PR-less task that pushed a branch. When present, * the provider fetches the changed files (`base...head`) so the session's changed-files diff --git a/src/vs/sessions/contrib/github/browser/pullRequestIconStatus.ts b/src/vs/sessions/contrib/github/browser/pullRequestIconStatus.ts index 5dbdb45a95612..b42d879cb28a0 100644 --- a/src/vs/sessions/contrib/github/browser/pullRequestIconStatus.ts +++ b/src/vs/sessions/contrib/github/browser/pullRequestIconStatus.ts @@ -5,8 +5,10 @@ import { IReaderWithStore } from '../../../../base/common/observable.js'; import { ThemeIcon } from '../../../../base/common/themables.js'; +import { IGitHubInfo } from '../../../services/sessions/common/session.js'; import { computePullRequestIcon, GitHubCIOverallStatus, GitHubPullRequestState, IGitHubPullRequest, IPullRequestIconStatus } from '../common/types.js'; import { IGitHubService } from './githubService.js'; +import { IPullRequestIconCache } from './pullRequestIconCache.js'; /** * Reads the live {@link IPullRequestIconStatus} for a pull request from the shared @@ -37,3 +39,24 @@ export function computeLivePullRequestIcon(reader: IReaderWithStore, gitHubServi const status = computePullRequestIconStatus(reader, gitHubService, owner, repo, livePR); return computePullRequestIcon(livePR.isDraft ? 'draft' : livePR.state, status); } + +/** + * Computes a session PR icon from the shared live model, with persistent and provider-reported fallbacks while it loads. + */ +export function computeSessionPullRequestIcon(reader: IReaderWithStore, gitHubService: IGitHubService, iconCache: IPullRequestIconCache, gitHubInfo: IGitHubInfo): ThemeIcon | undefined { + const pullRequest = gitHubInfo.pullRequest; + if (!pullRequest) { + return undefined; + } + + const prLink = pullRequest.uri.toString(); + const prModelRef = reader.store.add(gitHubService.createPullRequestModelReference(gitHubInfo.owner, gitHubInfo.repo, pullRequest.number)); + const livePullRequest = prModelRef.object.pullRequest.read(reader); + if (!livePullRequest) { + return iconCache.get(prLink) ?? pullRequest.icon ?? computePullRequestIcon(GitHubPullRequestState.Open); + } + + const icon = computeLivePullRequestIcon(reader, gitHubService, gitHubInfo.owner, gitHubInfo.repo, livePullRequest); + iconCache.set(prLink, icon); + return icon; +} diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index c08d6860399c0..8df5c254fc25f 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -50,8 +50,7 @@ import { ChatInteractivity, ChatOriginKind, DEFAULT_CHAT_CAPABILITIES, effective import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; import { IDeleteChatOptions, ISendRequestOptions, ISessionChangeEvent, ISessionModelPickerOptions, ISessionModelsSnapshot } from '../../../../services/sessions/common/sessionsProvider.js'; import { IGitHubService } from '../../../github/browser/githubService.js'; -import { computeLivePullRequestIcon } from '../../../github/browser/pullRequestIconStatus.js'; -import { computePullRequestIcon, GitHubPullRequestState } from '../../../github/common/types.js'; +import { computeSessionPullRequestIcon } from '../../../github/browser/pullRequestIconStatus.js'; import { IPullRequestIconCache } from '../../../github/browser/pullRequestIconCache.js'; import { mapProtocolStatus } from './agentHostDiffs.js'; import { createChangesets } from './agentHostSessionChangesets.js'; @@ -610,35 +609,11 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { return baseGitHubInfo; } - const prLink = baseGitHubInfo.pullRequest.uri.toString(); - const prModelRef = reader.store.add(this._gitHubService.createPullRequestModelReference( - baseGitHubInfo.owner, baseGitHubInfo.repo, baseGitHubInfo.pullRequest.number)); - const livePR = prModelRef.object.pullRequest.read(reader); - - if (!livePR) { - // The live model hasn't been fetched yet (e.g. right after a PR is first - // detected, or right after startup). Show the last known icon from the - // persistent cache, falling back to a neutral open-PR icon, so the row - // surfaces a PR icon immediately instead of the read/unread dot while the - // first fetch is in flight. The agent-host git state carries no PR state, - // so the live model refines it (merged/closed/draft/failing checks) within - // a poll cycle. - const icon = this._pullRequestIconCache.get(prLink) ?? computePullRequestIcon(GitHubPullRequestState.Open); - return { - ...baseGitHubInfo, - pullRequest: { ...baseGitHubInfo.pullRequest, icon } - }; - } - - const icon = computeLivePullRequestIcon(reader, this._gitHubService, baseGitHubInfo.owner, baseGitHubInfo.repo, livePR); - // Remember the freshly computed icon so the next startup can show it instantly. - // The cache de-duplicates unchanged icons, so this is a no-op when nothing changed. - this._pullRequestIconCache.set(prLink, icon); return { ...baseGitHubInfo, pullRequest: { ...baseGitHubInfo.pullRequest, - icon + icon: computeSessionPullRequestIcon(reader, this._gitHubService, this._pullRequestIconCache, baseGitHubInfo) } }; }); diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/COPILOT_CHAT_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/copilotChatSessions/COPILOT_CHAT_SESSIONS_PROVIDER.md index 1078cb405408b..05a34168cbd5c 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/COPILOT_CHAT_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/COPILOT_CHAT_SESSIONS_PROVIDER.md @@ -66,6 +66,9 @@ Adapts an existing `IAgentSession` from the chat layer into the `ISession` facad - Constructs with initial values from the agent session's metadata and timing - `update(session)` performs a batched observable transaction to update all reactive properties - Extracts workspace info, changes, description, and GitHub info from session metadata +- Treats a cloud provider's `pullRequestUrl` as authoritative for owner, repository, and PR number (including URL-only metadata), and falls back to resolving the PR from `owner`/`name`/`branch` when the URL is absent +- PR-less task cards must carry `diffRefs.headRef` into their session metadata as `branch`; repository-only metadata cannot drive the branch fallback +- Uses the shared GitHub PR model, background polling contribution, and persistent icon cache for github.com; provider-reported GitHub Enterprise links retain their provider state icon but skip the public `api.github.com` polling path - Maps `ChatSessionStatus` → `SessionStatus` - Handles both CLI and Cloud session metadata formats for repository resolution diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts index f0fdcf81ff830..59a32cf5cc1dc 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts @@ -10,7 +10,7 @@ import { CancellationError } from '../../../../../base/common/errors.js'; import { IMarkdownString, MarkdownString, markdownStringEqual } from '../../../../../base/common/htmlContent.js'; import { Disposable, DisposableStore, IDisposable, DisposableMap, MutableDisposable } from '../../../../../base/common/lifecycle.js'; import { Schemas } from '../../../../../base/common/network.js'; -import { autorun, constObservable, derived, derivedOpts, IObservable, IObservableSignal, IReader, ISettableObservable, ITransaction, observableSignal, observableValue, observableValueOpts, transaction } from '../../../../../base/common/observable.js'; +import { autorun, constObservable, derived, derivedOpts, IObservable, IObservableSignal, IReader, ISettableObservable, ITransaction, observableFromPromise, observableSignal, observableValue, observableValueOpts, transaction } from '../../../../../base/common/observable.js'; import { ThemeIcon } from '../../../../../base/common/themables.js'; import { URI } from '../../../../../base/common/uri.js'; import { ICommandService } from '../../../../../platform/commands/common/commands.js'; @@ -47,7 +47,8 @@ import { ClaudePreferAgentHostAgentsSettingId } from '../../../../../platform/ag import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { IGitHubService } from '../../../github/browser/githubService.js'; import { computePullRequestIcon, GitHubPullRequestState } from '../../../github/common/types.js'; -import { computeLivePullRequestIcon } from '../../../github/browser/pullRequestIconStatus.js'; +import { computeSessionPullRequestIcon } from '../../../github/browser/pullRequestIconStatus.js'; +import { IPullRequestIconCache } from '../../../github/browser/pullRequestIconCache.js'; import { structuralEquals } from '../../../../../base/common/equals.js'; import { CopilotCLISessionType } from '../../agentHost/browser/baseAgentHostSessionsProvider.js'; import { createChangesets } from './copilotChatSessionsChangesets.js'; @@ -320,6 +321,7 @@ class CopilotCLISession extends Disposable implements ICopilotChatSession { @IChatSessionsService private readonly chatSessionsService: IChatSessionsService, @IGitService private readonly gitService: IGitService, @IGitHubService private readonly gitHubService: IGitHubService, + @IPullRequestIconCache private readonly pullRequestIconCache: IPullRequestIconCache, @IStorageService private readonly storageService: IStorageService, @IConfigurationService private readonly configurationService: IConfigurationService, ) { @@ -538,7 +540,7 @@ class CopilotCLISession extends Disposable implements ICopilotChatSession { update(agentSession: IAgentSession): void { transaction((tx) => { - const session = new AgentSessionAdapter(agentSession, this.providerId, this.gitHubService); + const session = new AgentSessionAdapter(agentSession, this.providerId, this.gitHubService, this.pullRequestIconCache); this._workspaceData.set(session.workspace.get(), tx); this._title.set(session.title.get(), tx); this._status.set(session.status.get(), tx); @@ -1025,6 +1027,9 @@ class AgentSessionAdapter implements ICopilotChatSession { readonly lastTurnEnd: IObservable; private readonly _baseGitHubInfo: ReturnType>; + private readonly _pullRequestBranch: ReturnType>; + private readonly _pullRequestNumberFromBranch: IObservable | undefined>; + private readonly _pullRequestNumberCache = new Map>(); readonly gitHubInfo: IObservable; readonly permissionLevel: IObservable = constObservable(ChatPermissionLevel.Default); @@ -1039,6 +1044,7 @@ class AgentSessionAdapter implements ICopilotChatSession { session: IAgentSession, providerId: string, private readonly _gitHubService: IGitHubService, + private readonly _pullRequestIconCache: IPullRequestIconCache, ) { this.sessionId = toSessionId(providerId, session.resource); this.resource = session.resource; @@ -1048,17 +1054,49 @@ class AgentSessionAdapter implements ICopilotChatSession { this.createdAt = new Date(session.timing.created); this._baseGitHubInfo = observableValue(this, this._extractGitHubInfo(session)); - this.gitHubInfo = derived(this, reader => { + this._pullRequestBranch = observableValue(this, this._extractPullRequestBranch(session)); + this._pullRequestNumberFromBranch = derived(this, reader => { const base = this._baseGitHubInfo.read(reader); - if (!base?.pullRequest || !this._gitHubService) { - return base; + const branch = this._pullRequestBranch.read(reader); + if (base?.pullRequest || !base || !branch) { + return undefined; + } + return this._pullRequestNumberForBranch(base.owner, base.repo, branch); + }); + this.gitHubInfo = derived(this, reader => { + let info = this._baseGitHubInfo.read(reader); + if (!info) { + return undefined; } - const prModelRef = reader.store.add(this._gitHubService.createPullRequestModelReference(base.owner, base.repo, base.pullRequest.number)); - const livePR = prModelRef.object.pullRequest.read(reader); - if (!livePR) { - return base; + + if (!info.pullRequest) { + const pullRequestNumber = this._pullRequestNumberFromBranch.read(reader)?.read(reader).value; + if (pullRequestNumber === undefined) { + return info; + } + info = { + ...info, + pullRequest: { + number: pullRequestNumber, + uri: URI.parse(`https://github.com/${info.owner}/${info.repo}/pull/${pullRequestNumber}`), + } + }; + } + + const pullRequest = info.pullRequest; + if (!pullRequest) { + return info; } - return { ...base, pullRequest: { ...base.pullRequest, icon: computeLivePullRequestIcon(reader, this._gitHubService, base.owner, base.repo, livePR) } }; + if (pullRequest.uri.authority.toLowerCase() !== 'github.com') { + return info; + } + return { + ...info, + pullRequest: { + ...pullRequest, + icon: computeSessionPullRequestIcon(reader, this._gitHubService, this._pullRequestIconCache, info) + } + }; }); this._workspace = observableValue(this, this._buildWorkspace(session)); @@ -1119,6 +1157,8 @@ class AgentSessionAdapter implements ICopilotChatSession { update(session: IAgentSession): boolean { let changed = false; transaction(tx => { + const gitHubInfo = this._extractGitHubInfo(session); + const pullRequestBranch = this._extractPullRequestBranch(session); changed = setIfChanged(this._title, session.label, tx) || changed; changed = setIfChanged(this._workspace, this._buildWorkspace(session), tx, sessionWorkspaceEqual) || changed; const updatedTime = session.timing.lastRequestEnded ?? session.timing.lastRequestStarted ?? session.timing.created; @@ -1130,11 +1170,30 @@ class AgentSessionAdapter implements ICopilotChatSession { changed = setIfChanged(this._isRead, session.isRead(), tx) || changed; changed = setIfChanged(this._description, this._extractDescription(session), tx, markdownStringEquals) || changed; changed = setIfChanged(this._lastTurnEnd, session.timing.lastRequestEnded ? new Date(session.timing.lastRequestEnded) : undefined, tx, dateEquals) || changed; - changed = setIfChanged(this._baseGitHubInfo, this._extractGitHubInfo(session), tx, gitHubInfoEqual) || changed; + changed = setIfChanged(this._baseGitHubInfo, gitHubInfo, tx, gitHubInfoEqual) || changed; + changed = setIfChanged(this._pullRequestBranch, pullRequestBranch, tx) || changed; }); return changed; } + private _pullRequestNumberForBranch(owner: string, repo: string, branch: string): IObservable<{ readonly value?: number | undefined }> { + const key = `${owner}/${repo}@${branch}`; + const cached = this._pullRequestNumberCache.get(key); + if (cached) { + return cached; + } + + const lookup = this._gitHubService.findPullRequestNumberByHeadBranch(owner, repo, branch); + const observable = observableFromPromise(lookup); + this._pullRequestNumberCache.set(key, observable); + lookup.then(pullRequestNumber => { + if (pullRequestNumber === undefined && this._pullRequestNumberCache.get(key) === observable) { + this._pullRequestNumberCache.delete(key); + } + }); + return observable; + } + private _getSessionTypeIcon(session: IAgentSession): ThemeIcon { switch (session.providerType) { case AgentSessionProviders.Background: @@ -1161,18 +1220,14 @@ class AgentSessionAdapter implements ICopilotChatSession { return undefined; } - const { owner, repo } = this._extractOwnerRepo(session); + const pullRequestUri = this._extractPullRequestUri(session); + const pullRequestIdentity = pullRequestUri ? this._extractPullRequestIdentity(pullRequestUri) : undefined; + const { owner, repo } = pullRequestIdentity ?? this._extractOwnerRepo(session); if (!owner || !repo) { return undefined; } - const pullRequestUri = this._extractPullRequestUri(session); - if (!pullRequestUri) { - return { owner, repo }; - } - - const prNumber = this._extractPullRequestNumber(session, pullRequestUri); - if (prNumber === undefined) { + if (!pullRequestUri || !pullRequestIdentity) { return { owner, repo }; } @@ -1185,7 +1240,7 @@ class AgentSessionAdapter implements ICopilotChatSession { owner, repo, pullRequest: { - number: prNumber, + number: pullRequestIdentity.number, uri: pullRequestUri, icon, baseRefOid, @@ -1194,16 +1249,26 @@ class AgentSessionAdapter implements ICopilotChatSession { }; } - private _extractPullRequestNumber(session: IAgentSession, pullRequestUri: URI): number | undefined { - const metadata = session.metadata; - if (typeof metadata?.pullRequestNumber === 'number') { - return metadata.pullRequestNumber as number; + private _extractPullRequestBranch(session: IAgentSession): string | undefined { + if (session.providerType !== AgentSessionProviders.Cloud) { + return undefined; } - const match = /\/pull\/(\d+)/.exec(pullRequestUri.path); - if (match) { - return parseInt(match[1], 10); + if (typeof session.metadata?.host === 'string' && session.metadata.host.toLowerCase() !== 'github.com') { + return undefined; } - return undefined; + return typeof session.metadata?.branch === 'string' ? session.metadata.branch : undefined; + } + + private _extractPullRequestIdentity(pullRequestUri: URI): { readonly owner: string; readonly repo: string; readonly number: number } | undefined { + const match = /^\/(?[^/]+)\/(?[^/]+)\/pull\/(?\d+)\/?$/.exec(pullRequestUri.path); + if (!match?.groups) { + return undefined; + } + return { + owner: decodeURIComponent(match.groups.owner), + repo: decodeURIComponent(match.groups.repo), + number: parseInt(match.groups.number, 10), + }; } private _extractOwnerRepo(session: IAgentSession): { owner: string | undefined; repo: string | undefined } { @@ -1234,14 +1299,6 @@ class AgentSessionAdapter implements ICopilotChatSession { } } - // Parse from pullRequestUrl - if (typeof metadata.pullRequestUrl === 'string') { - const match = /github\.com\/([^/]+)\/([^/]+)\/pull\/(\d+)/.exec(metadata.pullRequestUrl as string); - if (match) { - return { owner: match[1], repo: match[2] }; - } - } - return { owner: undefined, repo: undefined }; } @@ -1387,6 +1444,9 @@ class AgentSessionAdapter implements ICopilotChatSession { } if (session.providerType === AgentSessionProviders.Cloud) { + if (typeof metadata.owner !== 'string' || typeof metadata.name !== 'string') { + return {}; + } const branch = typeof metadata.branch === 'string' ? metadata.branch : 'HEAD'; const repositoryUri = URI.from({ scheme: GITHUB_REMOTE_FILE_SCHEME, @@ -1558,6 +1618,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions @IAgentHostEnablementService private readonly agentHostEnablementService: IAgentHostEnablementService, @ILogService private readonly logService: ILogService, @IGitHubService private readonly gitHubService: IGitHubService, + @IPullRequestIconCache private readonly pullRequestIconCache: IPullRequestIconCache, @ILabelService private readonly labelService: ILabelService, @IChatModeService private readonly chatModeService: IChatModeService, @IUriIdentityService private readonly uriIdentityService: IUriIdentityService, @@ -2851,7 +2912,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions sessionsToMarkUnread.push(session); } } else { - const adapter = new AgentSessionAdapter(session, this.id, this.gitHubService); + const adapter = new AgentSessionAdapter(session, this.id, this.gitHubService, this.pullRequestIconCache); this._sessionCache.set(key, adapter); addedData.push(adapter); cacheChanged = true; diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts index 2627fc20ba54c..35f6b8dd85c03 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts @@ -7,11 +7,11 @@ import assert from 'assert'; import { Codicon } from '../../../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; import { timeout } from '../../../../../../base/common/async.js'; -import { DisposableStore, IDisposable, toDisposable } from '../../../../../../base/common/lifecycle.js'; +import { DisposableStore, IDisposable, ImmortalReference, toDisposable } from '../../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../../base/common/uri.js'; import { generateUuid } from '../../../../../../base/common/uuid.js'; import { mock } from '../../../../../../base/test/common/mock.js'; -import { autorun } from '../../../../../../base/common/observable.js'; +import { autorun, observableValue } from '../../../../../../base/common/observable.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { ConfigurationTarget, IConfigurationService, IConfigurationValue } from '../../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; @@ -46,6 +46,10 @@ import { extUri } from '../../../../../../base/common/resources.js'; import { CopilotCLISessionType } from '../../../agentHost/browser/baseAgentHostSessionsProvider.js'; import { IAgentHostEnablementService } from '../../../../../../platform/agentHost/common/agentHostEnablementService.js'; import { MockContextKeyService } from '../../../../../../platform/keybinding/test/common/mockKeybindingService.js'; +import { IGitHubService } from '../../../../github/browser/githubService.js'; +import { GitHubPullRequestModel } from '../../../../github/browser/models/githubPullRequestModel.js'; +import { IPullRequestIconCache } from '../../../../github/browser/pullRequestIconCache.js'; +import { computePullRequestIcon, GitHubPullRequestState, IGitHubPullRequest } from '../../../../github/common/types.js'; // ---- Helpers ---------------------------------------------------------------- @@ -148,12 +152,79 @@ interface ICreateProviderOptions { readonly commandExecutions?: IExecutedCommand[]; readonly getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined; readonly languageModelsService?: Partial; + readonly gitHubService?: IGitHubService; + readonly pullRequestIconCache?: IPullRequestIconCache; } function isCommandSessionItem(item: unknown): item is { readonly resource: URI; readonly label?: string } { return typeof item === 'object' && item !== null && 'resource' in item && URI.isUri(item.resource); } +class TestPullRequestIconCache implements IPullRequestIconCache { + + declare readonly _serviceBrand: undefined; + + private readonly _icons = new Map>(); + + get(prLink: string): ReturnType | undefined { + return this._icons.get(prLink); + } + + set(prLink: string, icon: ReturnType): void { + this._icons.set(prLink, icon); + } +} + +class TestGitHubService extends mock() { + + private readonly _pullRequest = observableValue(this, undefined); + private readonly _pullRequestModel: GitHubPullRequestModel; + + lookupCalls = 0; + pullRequestModelReferenceCalls = 0; + + constructor(private readonly _pullRequestNumber?: number) { + super(); + const pullRequest = this._pullRequest; + this._pullRequestModel = new class extends mock() { + override readonly pullRequest = pullRequest; + }(); + } + + override findPullRequestNumberByHeadBranch = async (): Promise => { + this.lookupCalls++; + return this._pullRequestNumber; + }; + + override createPullRequestModelReference = () => { + this.pullRequestModelReferenceCalls++; + return new ImmortalReference(this._pullRequestModel); + }; + + setPullRequest(pullRequest: IGitHubPullRequest): void { + this._pullRequest.set(pullRequest, undefined); + } +} + +function createPullRequest(state: GitHubPullRequestState, isDraft = false): IGitHubPullRequest { + return { + number: 42, + title: 'Cloud PR', + body: '', + state, + author: { login: 'owner', avatarUrl: '' }, + headRef: 'feature', + headSha: 'head', + baseRef: 'main', + isDraft, + createdAt: '', + updatedAt: '', + mergedAt: state === GitHubPullRequestState.Merged ? '' : undefined, + mergeable: undefined, + mergeableState: '', + }; +} + // ---- Provider factory ------------------------------------------------------- function createProvider( @@ -239,6 +310,8 @@ function createProviderWithConfig( }); instantiationService.stub(IUriIdentityService, { extUri }); instantiationService.stub(IAgentHostEnablementService, { _serviceBrand: undefined, enabled: opts?.agentHostEnabled ?? true }); + instantiationService.stub(IGitHubService, opts?.gitHubService ?? new TestGitHubService()); + instantiationService.stub(IPullRequestIconCache, opts?.pullRequestIconCache ?? new TestPullRequestIconCache()); const provider = disposables.add(instantiationService.createInstance(CopilotChatSessionsProvider)); return { provider, configService }; @@ -315,6 +388,8 @@ function createProviderForSendTests( instantiationService.stub(IUriIdentityService, { extUri }); instantiationService.stub(IAgentHostEnablementService, { _serviceBrand: undefined, enabled: opts?.agentHostEnabled ?? true }); instantiationService.stub(IContextKeyService, new MockContextKeyService()); + instantiationService.stub(IGitHubService, new TestGitHubService()); + instantiationService.stub(IPullRequestIconCache, new TestPullRequestIconCache()); return disposables.add(instantiationService.createInstance(CopilotChatSessionsProvider)); } @@ -842,6 +917,272 @@ suite('CopilotChatSessionsProvider', () => { assert.strictEqual(sessions[0].capabilities.get().supportsMultipleChats, false); }); + test('cloud session reports the provider pull request and uses the cached icon while live data loads', () => { + const resource = URI.from({ scheme: AgentSessionProviders.Cloud, path: '/session-1' }); + const gitHubService = new TestGitHubService(7); + const iconCache = new TestPullRequestIconCache(); + const prUri = URI.parse('https://github.com/owner/repo/pull/42'); + const cachedIcon = computePullRequestIcon(GitHubPullRequestState.Merged); + iconCache.set(prUri.toString(), cachedIcon); + model.addSession(createMockAgentSession(resource, { + providerType: AgentSessionProviders.Cloud, + metadata: { + owner: 'wrong-owner', + name: 'wrong-repo', + branch: 'feature', + pullRequestNumber: 7, + pullRequestUrl: prUri.toString(), + pullRequestState: GitHubPullRequestState.Open, + }, + })); + + const provider = createProvider(disposables, model, { gitHubService, pullRequestIconCache: iconCache }); + const gitHubInfo = provider.getSessions()[0].workspace.get()?.folders[0]?.gitRepository?.gitHubInfo.get(); + + assert.deepStrictEqual({ + owner: gitHubInfo?.owner, + repo: gitHubInfo?.repo, + pullRequest: gitHubInfo?.pullRequest && { + number: gitHubInfo.pullRequest.number, + uri: gitHubInfo.pullRequest.uri.toString(), + icon: gitHubInfo.pullRequest.icon, + }, + lookupCalls: gitHubService.lookupCalls, + pullRequestModelReferenceCalls: gitHubService.pullRequestModelReferenceCalls, + }, { + owner: 'owner', + repo: 'repo', + pullRequest: { + number: 42, + uri: prUri.toString(), + icon: cachedIcon, + }, + lookupCalls: 0, + pullRequestModelReferenceCalls: 1, + }); + }); + + test('cloud session accepts pull request URL-only metadata without creating an invalid workspace URI', () => { + const resource = URI.from({ scheme: AgentSessionProviders.Cloud, path: '/session-1' }); + const gitHubService = new TestGitHubService(); + model.addSession(createMockAgentSession(resource, { + providerType: AgentSessionProviders.Cloud, + metadata: { + pullRequestUrl: 'https://github.com/owner/repo/pull/42', + pullRequestState: GitHubPullRequestState.Open, + }, + })); + + const provider = createProvider(disposables, model, { gitHubService }); + const workspace = provider.getSessions()[0].workspace.get(); + const gitHubInfo = workspace?.folders[0]?.gitRepository?.gitHubInfo.get(); + + assert.deepStrictEqual({ + workspaceRoot: workspace?.folders[0]?.root.toString(), + owner: gitHubInfo?.owner, + repo: gitHubInfo?.repo, + pullRequest: gitHubInfo?.pullRequest && { + number: gitHubInfo.pullRequest.number, + uri: gitHubInfo.pullRequest.uri.toString(), + }, + }, { + workspaceRoot: URI.parse('unknown:///').toString(), + owner: 'owner', + repo: 'repo', + pullRequest: { + number: 42, + uri: 'https://github.com/owner/repo/pull/42', + }, + }); + }); + + test('cloud session keeps provider-reported enterprise PR identity without public GitHub polling', () => { + const resource = URI.from({ scheme: AgentSessionProviders.Cloud, path: '/session-1' }); + const gitHubService = new TestGitHubService(7); + model.addSession(createMockAgentSession(resource, { + providerType: AgentSessionProviders.Cloud, + metadata: { + owner: 'wrong-owner', + name: 'wrong-repo', + host: 'github.example.com', + branch: 'feature', + pullRequestNumber: 7, + pullRequestUrl: 'https://github.example.com/owner/repo/pull/42', + pullRequestState: GitHubPullRequestState.Open, + }, + })); + + const provider = createProvider(disposables, model, { gitHubService }); + const gitHubInfo = provider.getSessions()[0].workspace.get()?.folders[0]?.gitRepository?.gitHubInfo.get(); + + assert.deepStrictEqual({ + owner: gitHubInfo?.owner, + repo: gitHubInfo?.repo, + pullRequest: gitHubInfo?.pullRequest && { + number: gitHubInfo.pullRequest.number, + uri: gitHubInfo.pullRequest.uri.toString(), + icon: gitHubInfo.pullRequest.icon, + }, + lookupCalls: gitHubService.lookupCalls, + pullRequestModelReferenceCalls: gitHubService.pullRequestModelReferenceCalls, + }, { + owner: 'owner', + repo: 'repo', + pullRequest: { + number: 42, + uri: 'https://github.example.com/owner/repo/pull/42', + icon: computePullRequestIcon(GitHubPullRequestState.Open), + }, + lookupCalls: 0, + pullRequestModelReferenceCalls: 0, + }); + }); + + test('cloud session infers a provider-omitted pull request from its branch and updates the live icon', async () => { + const resource = URI.from({ scheme: AgentSessionProviders.Cloud, path: '/session-1' }); + const gitHubService = new TestGitHubService(42); + const iconCache = new TestPullRequestIconCache(); + model.addSession(createMockAgentSession(resource, { + providerType: AgentSessionProviders.Cloud, + metadata: { + owner: 'owner', + name: 'repo', + branch: 'feature', + }, + })); + + const provider = createProvider(disposables, model, { gitHubService, pullRequestIconCache: iconCache }); + const gitHubInfoObs = provider.getSessions()[0].workspace.get()!.folders[0].gitRepository!.gitHubInfo; + const firstObservation = disposables.add(autorun(reader => gitHubInfoObs.read(reader))); + await timeout(0); + const beforeLiveUpdate = gitHubInfoObs.get()?.pullRequest; + + gitHubService.setPullRequest(createPullRequest(GitHubPullRequestState.Merged)); + const afterLiveUpdate = gitHubInfoObs.get()?.pullRequest; + firstObservation.dispose(); + + let firstReobservedNumber: number | undefined; + let captured = false; + const secondObservation = autorun(reader => { + const pullRequestNumber = gitHubInfoObs.read(reader)?.pullRequest?.number; + if (!captured) { + firstReobservedNumber = pullRequestNumber; + captured = true; + } + }); + disposables.add(secondObservation); + model.replaceSession(createMockAgentSession(resource, { + providerType: AgentSessionProviders.Cloud, + title: 'Updated Cloud Session', + metadata: { + owner: 'owner', + name: 'repo', + branch: 'feature', + }, + })); + + assert.deepStrictEqual({ + beforeLiveUpdate: beforeLiveUpdate && { + number: beforeLiveUpdate.number, + uri: beforeLiveUpdate.uri.toString(), + icon: beforeLiveUpdate.icon, + }, + afterLiveUpdate: afterLiveUpdate && { + number: afterLiveUpdate.number, + uri: afterLiveUpdate.uri.toString(), + icon: afterLiveUpdate.icon, + }, + lookupCalls: gitHubService.lookupCalls, + cachedIcon: iconCache.get('https://github.com/owner/repo/pull/42'), + firstReobservedNumber, + numberAfterUpdate: gitHubInfoObs.get()?.pullRequest?.number, + }, { + beforeLiveUpdate: { + number: 42, + uri: 'https://github.com/owner/repo/pull/42', + icon: computePullRequestIcon(GitHubPullRequestState.Open), + }, + afterLiveUpdate: { + number: 42, + uri: 'https://github.com/owner/repo/pull/42', + icon: computePullRequestIcon(GitHubPullRequestState.Merged), + }, + lookupCalls: 1, + cachedIcon: computePullRequestIcon(GitHubPullRequestState.Merged), + firstReobservedNumber: 42, + numberAfterUpdate: 42, + }); + }); + + test('cloud session waits for provider PR metadata after an unsuccessful branch lookup without polling on unrelated updates', async () => { + const resource = URI.from({ scheme: AgentSessionProviders.Cloud, path: '/session-1' }); + const gitHubService = new TestGitHubService(); + const metadata = { + owner: 'owner', + name: 'repo', + branch: 'feature', + }; + model.addSession(createMockAgentSession(resource, { providerType: AgentSessionProviders.Cloud, metadata })); + + const provider = createProvider(disposables, model, { gitHubService }); + const gitHubInfoObs = provider.getSessions()[0].workspace.get()!.folders[0].gitRepository!.gitHubInfo; + disposables.add(autorun(reader => gitHubInfoObs.read(reader))); + await timeout(0); + model.replaceSession(createMockAgentSession(resource, { + providerType: AgentSessionProviders.Cloud, + title: 'Updated Cloud Session', + metadata, + })); + await timeout(0); + + model.replaceSession(createMockAgentSession(resource, { + providerType: AgentSessionProviders.Cloud, + metadata: { + ...metadata, + pullRequestUrl: 'https://github.com/owner/repo/pull/42', + }, + })); + + assert.deepStrictEqual({ + lookupCalls: gitHubService.lookupCalls, + pullRequest: gitHubInfoObs.get()?.pullRequest && { + number: gitHubInfoObs.get()!.pullRequest!.number, + uri: gitHubInfoObs.get()!.pullRequest!.uri.toString(), + }, + }, { + lookupCalls: 1, + pullRequest: { + number: 42, + uri: 'https://github.com/owner/repo/pull/42', + }, + }); + }); + + test('non-cloud sessions do not infer pull requests from branch metadata', async () => { + const resource = URI.from({ scheme: AgentSessionProviders.Background, path: '/session-1' }); + const gitHubService = new TestGitHubService(42); + model.addSession(createMockAgentSession(resource, { + metadata: { + owner: 'owner', + name: 'repo', + branch: 'feature', + }, + })); + + const provider = createProvider(disposables, model, { gitHubService }); + const gitHubInfoObs = provider.getSessions()[0].workspace.get()!.folders[0].gitRepository!.gitHubInfo; + disposables.add(autorun(reader => gitHubInfoObs.read(reader))); + await timeout(0); + + assert.deepStrictEqual({ + lookupCalls: gitHubService.lookupCalls, + pullRequest: gitHubInfoObs.get()?.pullRequest, + }, { + lookupCalls: 0, + pullRequest: undefined, + }); + }); + test('copilot CLI sessions do not have supportsMultipleChats when setting is disabled', () => { const resource = URI.from({ scheme: AgentSessionProviders.Background, path: '/session-1' }); model.addSession(createMockAgentSession(resource)); @@ -1829,4 +2170,3 @@ suite('CopilotChatSessionsProvider', () => { ); }); }); -