Sync changes action for git synchronization in agent sessions#298018
Sync changes action for git synchronization in agent sessions#298018
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Sessions-window contribution that surfaces Git ahead/behind status for the active session worktree and exposes a “sync changes” toolbar action in the Sessions Changes view.
Changes:
- Import a new
gitSynccontribution into the Sessions desktop entrypoint. - Add
GitSyncContributionto track the active session worktree repository state and conditionally register a sync action. - Update the Changes view toolbar button config to render the new sync action consistently.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/sessions/sessions.desktop.main.ts | Wires the new git-sync contribution into the Sessions desktop app startup. |
| src/vs/sessions/contrib/gitSync/browser/gitSync.contribution.ts | New workbench contribution that observes repo HEAD upstream ahead/behind and registers a toolbar action for Git sync. |
| src/vs/sessions/contrib/changesView/browser/changesView.ts | Adds button rendering config for the new sync action in the Changes view toolbar. |
Comments suppressed due to low confidence (4)
src/vs/sessions/contrib/gitSync/browser/gitSync.contribution.ts:52
- The openRepository(worktreeUri) promise chain doesn’t handle rejection. If opening the repository fails, this will surface as an unhandled promise rejection and can leave stale UI state. Add a .catch (or async/await + try/catch) that clears the action and resets the context key.
this.gitService.openRepository(worktreeUri).then(repository => {
if (!repository) {
src/vs/sessions/contrib/gitSync/browser/gitSync.contribution.ts:44
- When the active session changes to a different worktree, the previous ahead/behind context key and toolbar action can remain visible until openRepository resolves and the repo autorun runs. To avoid briefly showing stale counts/actions for the prior session, clear the existing action and reset the context key before starting the async openRepository flow for the new session.
this._register(autorun(reader => {
const activeSession = this.sessionManagementService.activeSession.read(reader);
this._gitRepoDisposables.clear();
const worktreeUri = activeSession ? this.sessionManagementService.getActiveSession()?.worktree : undefined;
src/vs/sessions/contrib/gitSync/browser/gitSync.contribution.ts:112
- Executing 'git.sync' without an argument can cause the Git extension to prompt for a repository (it falls back to pickRepository when it can't infer one from args[0]) or sync the wrong repo when multiple are open. Since this action is scoped to the active session worktree, pass a repository-identifying argument (e.g. the worktree URI or repository.sourceControl) when executing the command.
override async run(accessor: ServicesAccessor): Promise<void> {
const commandService = accessor.get(ICommandService);
await commandService.executeCommand('git.sync');
}
src/vs/sessions/contrib/gitSync/browser/gitSync.contribution.ts:84
- The computed title adds a trailing space when there are only behind commits (e.g. "2↓ "). Consider building the parts and joining with a single space or trimming the final string so button labels don't contain extra whitespace.
let title = '';
if (behind > 0) {
title += `${behind}↓ `;
}
if (ahead > 0) {
| const behind = head.behind ?? 0; | ||
| const hasSyncChanges = ahead > 0 || behind > 0; | ||
| contextKey.set(hasSyncChanges); | ||
| this._syncActionDisposable.value = registerSyncAction(ahead, behind); |
There was a problem hiding this comment.
The call to registerSyncAction passes (ahead, behind) but registerSyncAction is declared as (behind, ahead). This will swap the counts in the title/tooltip and show incorrect ahead/behind indicators; pass arguments in the correct order (or rename/reorder the function parameters to match the call).
| this._syncActionDisposable.value = registerSyncAction(ahead, behind); | |
| this._syncActionDisposable.value = registerSyncAction(behind, ahead); |
| this.gitService.openRepository(worktreeUri).then(repository => { | ||
| if (!repository) { | ||
| this._syncActionDisposable.clear(); | ||
| contextKey.set(false); | ||
| return; |
There was a problem hiding this comment.
openRepository(worktreeUri) resolves asynchronously, but this flow isn’t guarded against the active session changing while the promise is in flight. A late resolution can attach repository autoruns for a stale worktree and update the context key/action for the wrong session. Consider using a CancellationTokenSource (as in newChatViewPane._openRepository) or capturing the current worktree URI and bailing out if it no longer matches when the promise resolves.
This issue also appears in the following locations of the same file:
- line 51
- line 40
Copilot Generated Description:Introduce a new action for synchronizing changes with Git in agent sessions, allowing users to see ahead and behind commits relative to their upstream. This includes the registration of the action and its integration into the changes view. Additionally, the necessary contributions for the new functionality are added to the main session file.