Skip to content

Agents - refactor GitHub models #313891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lszomoru merged 7 commits into from
May 2, 2026
Merged

Agents - refactor GitHub models #313891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6967fdb
Agents - scaffold the model reference collections
lszomoru May 1, 2026
1530f89
Add test coverage for the new methods
lszomoru May 1, 2026
637b246
Migrated existing tests to use the new code path
lszomoru May 1, 2026
7382c5a
Switch over to using the new references
lszomoru May 1, 2026
adbb4e2
More cleanup
lszomoru May 2, 2026
ff1cd55
Centralized polling of the models
lszomoru May 2, 2026
ac70aee
More fixes
lszomoru May 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 3 additions & 29 deletions src/vs/sessions/contrib/changes/browser/checksActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import { Codicon } from '../../../../base/common/codicons.js';
import { Disposable } from '../../../../base/common/lifecycle.js';
import { derived } from '../../../../base/common/observable.js';
import { localize, localize2 } from '../../../../nls.js';
import { Action2, MenuId, registerAction2 } from '../../../../platform/actions/common/actions.js';
import { ContextKeyExpr, IContextKeyService, RawContextKey } from '../../../../platform/contextkey/common/contextkey.js';
Expand Down Expand Up @@ -103,30 +102,12 @@ class ActiveSessionFailedCIChecksContextContribution extends Disposable implemen

constructor(
@IContextKeyService contextKeyService: IContextKeyService,
@ISessionsManagementService sessionManagementService: ISessionsManagementService,
@IGitHubService gitHubService: IGitHubService,
) {
super();

const ciModelObs = derived(this, reader => {
const session = sessionManagementService.activeSession.read(reader);
if (!session) {
return undefined;
}
const gitHubInfo = session.gitHubInfo.read(reader);
if (!gitHubInfo?.pullRequest) {
return undefined;
}
const prModel = gitHubService.getPullRequest(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number);
const pr = prModel.pullRequest.read(reader);
if (!pr) {
return undefined;
}
return gitHubService.getPullRequestCI(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number, pr.headSha);
});

this._register(bindContextKey(hasActiveSessionFailedCIChecks, contextKeyService, reader => {
const ciModel = ciModelObs.read(reader);
const ciModel = gitHubService.activeSessionPullRequestCIObs.read(reader);
if (!ciModel) {
return false;
}
Expand Down Expand Up @@ -167,18 +148,11 @@ class FixCIChecksAction extends Action2 {
return;
}

const gitHubInfo = activeSession.gitHubInfo.get();
if (!gitHubInfo?.pullRequest) {
return;
}

const prModel = gitHubService.getPullRequest(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number);
const pr = prModel.pullRequest.get();
if (!pr) {
const ciModel = gitHubService.activeSessionPullRequestCIObs.get();
if (!ciModel) {
return;
}

const ciModel = gitHubService.getPullRequestCI(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number, pr.headSha);
const checks = ciModel.checks.get();
const failedChecks = getFailedChecks(checks);
if (failedChecks.length === 0) {
Expand Down
45 changes: 3 additions & 42 deletions src/vs/sessions/contrib/changes/browser/checksViewModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { URI } from '../../../../base/common/uri.js';
import { IGitHubService } from '../../github/browser/githubService.js';
import { GitHubPullRequestCIModel } from '../../github/browser/models/githubPullRequestCIModel.js';
import { ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js';
import { structuralEquals } from '../../../../base/common/equals.js';
import { isEqual } from '../../../../base/common/resources.js';

export class ChecksViewModel extends Disposable {
readonly activeSessionResourceObs: IObservable<URI | undefined>;
Expand All @@ -21,52 +21,13 @@ export class ChecksViewModel extends Disposable {
) {
super();

this.activeSessionResourceObs = derived<URI | undefined>(this, reader => {
this.activeSessionResourceObs = derivedOpts<URI | undefined>({ equalsFn: isEqual }, reader => {
const session = sessionManagementService.activeSession.read(reader);
return session?.resource;
});

const pullRequestInfoObs = derivedOpts<{ owner: string; repo: string; prNumber: number; headSha: string } | undefined>({
equalsFn: structuralEquals
}, reader => {
const session = sessionManagementService.activeSession.read(reader);
if (!session) {
return undefined;
}

const gitHubInfo = session.gitHubInfo.read(reader);
if (!gitHubInfo?.pullRequest) {
return undefined;
}

const prModel = gitHubService.getPullRequest(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number);
const pr = prModel.pullRequest.read(reader);
if (!pr) {
return undefined;
}

return {
owner: gitHubInfo.owner,
repo: gitHubInfo.repo,
prNumber: gitHubInfo.pullRequest.number,
headSha: pr.headSha
};
});

this.checksObs = derived(this, reader => {
const pullRequestInfo = pullRequestInfoObs.read(reader);
if (!pullRequestInfo) {
return undefined;
}

// Use the PR's headSha (commit SHA) rather than the branch
// name so CI checks can still be fetched after branch deletion
// (e.g. after the PR is merged).
const ciModel = gitHubService.getPullRequestCI(pullRequestInfo.owner, pullRequestInfo.repo, pullRequestInfo.prNumber, pullRequestInfo.headSha);
ciModel.refresh();
reader.store.add(ciModel.startPolling());

return ciModel;
return gitHubService.activeSessionPullRequestCIObs.read(reader);
});
}
}
158 changes: 52 additions & 106 deletions src/vs/sessions/contrib/codeReview/browser/codeReviewService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { Disposable, DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js';
import { autorun, derivedOpts, IObservable, observableValue, transaction } from '../../../../base/common/observable.js';
import { Disposable } from '../../../../base/common/lifecycle.js';
import { autorun, derivedOpts, IObservable, ISettableObservable, observableValue, transaction } from '../../../../base/common/observable.js';
import { isEqual } from '../../../../base/common/resources.js';
import { URI, UriComponents } from '../../../../base/common/uri.js';
import { IRange, Range } from '../../../../editor/common/core/range.js';
Expand All @@ -19,7 +19,6 @@ import { isIChatSessionFileChange2 } from '../../../../workbench/contrib/chat/co
import { IGitHubService } from '../../github/browser/githubService.js';
import { ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js';
import { ISessionFileChange } from '../../../services/sessions/common/session.js';
import { structuralEquals } from '../../../../base/common/equals.js';

// --- Types -------------------------------------------------------------------

Expand Down Expand Up @@ -234,17 +233,11 @@ interface IStoredCodeReviewComment {
// --- Implementation ----------------------------------------------------------

interface ISessionReviewData {
readonly state: ReturnType<typeof observableValue<ICodeReviewState>>;
readonly state: ISettableObservable<ICodeReviewState>;
}

type IPullRequestReviewThreadsModel = ReturnType<IGitHubService['getPullRequestReviewThreads']>;

interface IPRSessionReviewData {
readonly state: ReturnType<typeof observableValue<IPRReviewState>>;
readonly disposables: DisposableStore;
readonly pollingDisposable: DisposableStore;
reviewThreadsModel?: IPullRequestReviewThreadsModel;
initialized: boolean;
readonly state: ISettableObservable<IPRReviewState>;
}

function isRawCodeReviewRangeWithPositions(range: IRawCodeReviewRange): range is IRawCodeReviewRangeWithPositions {
Expand Down Expand Up @@ -332,43 +325,60 @@ export class CodeReviewService extends Disposable implements ICodeReviewService
return this._sessionsManagementService.activeSession.read(reader)?.resource;
});

const gitHubInfoObs = derivedOpts<{ owner: string; repo: string; pullRequestNumber: number } | undefined>({ equalsFn: structuralEquals }, reader => {
const gitHubInfo = this._sessionsManagementService.activeSession.read(reader)?.gitHubInfo.read(reader);
if (!gitHubInfo?.pullRequest) {
return undefined;
}

return {
owner: gitHubInfo.owner,
repo: gitHubInfo.repo,
pullRequestNumber: gitHubInfo.pullRequest.number,
};
});

// Subscribe to the active session's PR review threads model and project
// review threads into per-session PR review state. The model lifetime is
// owned by `IGitHubService.activeSessionPullRequestReviewThreadsObs`; we
// only consume it here.
this._register(autorun(reader => {
const activeSessionResource = activeSessionResourceObs.read(reader);
if (!activeSessionResource) {
return;
}

const gitHubInfo = gitHubInfoObs.read(reader);
const data = this._ensurePRReviewInitialized(activeSessionResource, gitHubInfo);

if (!data.reviewThreadsModel) {
const reviewThreadsModel = this._gitHubService.activeSessionPullRequestReviewThreadsObs.read(reader);
if (!reviewThreadsModel) {
return;
}

// Initial fetch of review threads
data.reviewThreadsModel.refresh().catch(err => {
this._logService.error('[CodeReviewService] Failed to fetch PR review threads:', err);
data.state.set({ kind: PRReviewStateKind.Error, reason: String(err) }, undefined);
});
const data = this._getOrCreatePRReviewData(activeSessionResource);
if (data.state.read(undefined).kind === PRReviewStateKind.None) {
data.state.set({ kind: PRReviewStateKind.Loading }, undefined);
Comment thread
lszomoru marked this conversation as resolved.
}

// Start polling of review threads
data.pollingDisposable.add(data.reviewThreadsModel.startPolling());
const session = this._sessionsManagementService.getSession(activeSessionResource);
const workspace = session?.workspace.read(undefined);

// Map review threads -> local state.
reader.store.add(autorun(innerReader => {
const threads = reviewThreadsModel.reviewThreads.read(innerReader);
const converted = this._convertedPRCommentsBySession.get(activeSessionResource.toString());
const comments: IPRReviewComment[] = [];

for (const thread of threads) {
if (thread.isResolved) {
continue;
}
const threadId = String(thread.id);
if (converted?.has(threadId)) {
continue;
}
const baseUri = workspace?.repositories[0]?.workingDirectory ?? workspace?.repositories[0]?.uri;
if (!baseUri) {
continue;
}
const fileUri = URI.joinPath(baseUri, thread.path);
const line = thread.line ?? 1;
const firstComment = thread.comments[0];
comments.push({
id: String(thread.id),
uri: fileUri,
range: new Range(line, 1, line, 1),
body: firstComment?.body ?? '',
author: firstComment?.author.login ?? '',
});
}

reader.store.add(toDisposable(() => {
data.pollingDisposable.clear();
data.state.set({ kind: PRReviewStateKind.Loaded, comments }, undefined);
}));
}));
}
Expand Down Expand Up @@ -631,11 +641,13 @@ export class CodeReviewService extends Disposable implements ICodeReviewService
const session = this._sessionsManagementService.getSession(sessionResource);
const gitHubInfo = session?.gitHubInfo.get();
if (gitHubInfo?.pullRequest) {
const reviewThreadsModel = this._gitHubService.getPullRequestReviewThreads(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number);
const modelRef = this._gitHubService.createPullRequestReviewThreadsModelReference(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number);
try {
await reviewThreadsModel.resolveThread(threadId);
await modelRef.object.resolveThread(threadId);
} catch (err) {
this._logService.warn('[CodeReviewService] Failed to resolve PR thread on GitHub:', err);
} finally {
modelRef.dispose();
}
}

Expand Down Expand Up @@ -676,85 +688,19 @@ export class CodeReviewService extends Disposable implements ICodeReviewService
if (!data) {
data = {
state: observableValue<IPRReviewState>(`prReview.state.${key}`, { kind: PRReviewStateKind.None }),
disposables: new DisposableStore(),
pollingDisposable: new DisposableStore(),
initialized: false,
};
data.disposables.add(data.pollingDisposable);
this._prReviewBySession.set(key, data);
}
return data;
}

private _ensurePRReviewInitialized(sessionResource: URI, gitHubInfo: { owner: string; repo: string; pullRequestNumber: number } | undefined): IPRSessionReviewData {
const data = this._getOrCreatePRReviewData(sessionResource);
if (data.initialized) {
return data;
}

const session = this._sessionsManagementService.getSession(sessionResource);
if (!session || !gitHubInfo) {
return data;
}

data.initialized = true;
data.state.set({ kind: PRReviewStateKind.Loading }, undefined);

const reviewThreadsModel = this._gitHubService.getPullRequestReviewThreads(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequestNumber);
const workspace = session.workspace.get();
data.reviewThreadsModel = reviewThreadsModel;

// Watch the PR review threads model and map to local state
data.disposables.add(autorun(reader => {
const threads = reviewThreadsModel.reviewThreads.read(reader);
const converted = this._convertedPRCommentsBySession.get(sessionResource.toString());
const comments: IPRReviewComment[] = [];

for (const thread of threads) {
if (thread.isResolved) {
continue;
}
const threadId = String(thread.id);
if (converted?.has(threadId)) {
continue;
}
const baseUri = workspace?.repositories[0]?.workingDirectory ?? workspace?.repositories[0]?.uri;
if (!baseUri) {
continue;
}
const fileUri = URI.joinPath(baseUri, thread.path);
const line = thread.line ?? 1;
const firstComment = thread.comments[0];
comments.push({
id: String(thread.id),
uri: fileUri,
range: new Range(line, 1, line, 1),
body: firstComment?.body ?? '',
author: firstComment?.author.login ?? '',
});
}

data.state.set({ kind: PRReviewStateKind.Loaded, comments }, undefined);
}));

return data;
}

private _disposePRReview(sessionResource: URI): void {
const key = sessionResource.toString();
this._convertedPRCommentsBySession.delete(key);
const data = this._prReviewBySession.get(key);
if (data) {
data.disposables.dispose();

this._prReviewBySession.delete(key);
}
this._prReviewBySession.delete(key);
}

override dispose(): void {
for (const data of this._prReviewBySession.values()) {
data.disposables.dispose();
}
this._prReviewBySession.clear();

super.dispose();
Expand Down
Loading
Loading