Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions src/@types/vscode.proposed.chatParticipantPrivate.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,12 @@ declare module 'vscode' {
chatSessionResource?: Uri;
chatInteractionId?: string;
terminalCommand?: string;
/**
* The working directory URI for the session, if set.
* In the agents window, each session can have its own working directory
* that differs from the current workspace folders.
*/
workingDirectory?: Uri;
/**
* Unique ID for the subagent invocation, used to group tool calls from the same subagent run together.
*/
Expand Down Expand Up @@ -334,6 +340,12 @@ declare module 'vscode' {
chatRequestId?: string;
chatSessionResource?: Uri;
chatInteractionId?: string;
/**
* The working directory URI for the session, if set.
* In the agents window, each session can have its own working directory
* that differs from the current workspace folders.
*/
workingDirectory?: Uri;
/**
* If set, tells the tool that it should include confirmation messages.
*/
Expand Down
72 changes: 72 additions & 0 deletions src/test/view/reviewCommentController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@ class TestReviewCommentController extends ReviewCommentController {
public workspaceFileChangeCommentThreads() {
return this._workspaceFileChangeCommentThreads;
}
public obsoleteFileChangeCommentThreads() {
return this._obsoleteFileChangeCommentThreads;
}
public reviewSchemeFileChangeCommentThreads() {
return this._reviewSchemeFileChangeCommentThreads;
}
public seedWorkspaceThreads(path: string, threads: GHPRCommentThread[]) {
this._workspaceFileChangeCommentThreads[path] = threads;
}
public seedObsoleteThreads(path: string, threads: GHPRCommentThread[]) {
this._obsoleteFileChangeCommentThreads[path] = threads;
}
public callFindMatchingThread(thread: import('../../common/comment').IReviewThread) {
return (this as any)._findMatchingThread(thread);
}
}

describe('ReviewCommentController', function () {
Expand Down Expand Up @@ -340,4 +355,61 @@ describe('ReviewCommentController', function () {
assert.strictEqual(workspaceFileChangeCommentThreads[fileName].length, 1);
});
});

describe('_findMatchingThread', function () {
it('returns the moved thread when an existing workspace thread becomes outdated', function () {
const fileName = 'data/products.json';
const uri = vscode.Uri.parse(`${repository.rootUri.toString()}/${fileName}`);
const reviewModel = new ReviewModel();
const reviewCommentController = new TestReviewCommentController(
reviewManager,
manager,
repository,
reviewModel,
gitApiImpl,
telemetry,
);

const threadA = createGHPRCommentThread('thread-A', uri);
const threadB = createGHPRCommentThread('thread-B', uri);
const existingObsolete = createGHPRCommentThread('thread-C', uri);

reviewCommentController.seedWorkspaceThreads(fileName, [threadA, threadB]);
reviewCommentController.seedObsoleteThreads(fileName, [existingObsolete]);

const reviewThread: import('../../common/comment').IReviewThread = {
id: 'thread-B',
isResolved: false,
viewerCanResolve: false,
viewerCanUnresolve: false,
path: fileName,
diffSide: DiffSide.RIGHT,
startLine: 1,
endLine: 1,
originalStartLine: 1,
originalEndLine: 1,
isOutdated: true,
comments: [],
subjectType: SubjectType.LINE,
};

const result = reviewCommentController.callFindMatchingThread(reviewThread);

// The matching thread should be the moved thread, not `undefined` or the wrong one.
assert.strictEqual(result.threadMap, reviewCommentController.obsoleteFileChangeCommentThreads());
assert.ok(result.index > -1, 'index should point to the moved thread');
assert.strictEqual(result.threadMap[fileName][result.index], threadB);

// Workspace map no longer contains the moved thread.
const workspace = reviewCommentController.workspaceFileChangeCommentThreads();
assert.strictEqual(workspace[fileName].length, 1);
assert.strictEqual(workspace[fileName][0], threadA);

// Obsolete map now contains the moved thread appended after the pre-existing one.
const obsolete = reviewCommentController.obsoleteFileChangeCommentThreads();
assert.strictEqual(obsolete[fileName].length, 2);
assert.strictEqual(obsolete[fileName][0], existingObsolete);
assert.strictEqual(obsolete[fileName][1], threadB);
});
});
});
14 changes: 12 additions & 2 deletions src/view/pullRequestCommentController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,17 @@ export class PullRequestCommentController extends CommentControllerBase implemen
private async onDidChangeReviewThreads(e: ReviewThreadChangeEvent): Promise<void> {
for (const thread of e.added) {
const fileName = thread.path;
const key = this.getCommentThreadCacheKey(thread.path, thread.diffSide === DiffSide.LEFT);

// Defensive: if a comment thread for this review thread id already exists in the cache
// (e.g. because addThreadsForEditors already created it from the cache before this event was
// processed), update it in place instead of creating a duplicate VS Code comment thread.
const existing = this._commentThreadCache[key]?.find(t => t.gitHubThreadId === thread.id);
if (existing) {
updateThread(this._context, existing, thread, this._githubRepositories);
continue;
}

const index = this._pendingCommentThreadAdds.findIndex(t => {
const samePath = this._folderRepoManager.gitRelativeRootPath(t.uri.path) === thread.path;
const sameLine = (t.range === undefined && thread.subjectType === SubjectType.FILE) || (t.range && t.range.end.line + 1 === thread.endLine);
Expand Down Expand Up @@ -270,9 +281,8 @@ export class PullRequestCommentController extends CommentControllerBase implemen
}

if (!newThread) {
return;
continue;
}
Comment thread
alexr00 marked this conversation as resolved.
const key = this.getCommentThreadCacheKey(thread.path, thread.diffSide === DiffSide.LEFT);
if (this._commentThreadCache[key]) {
this._commentThreadCache[key].push(newThread);
} else {
Expand Down
20 changes: 17 additions & 3 deletions src/view/reviewCommentController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,17 @@ export class ReviewCommentController extends CommentControllerBase implements Co
for (const thread of e.added) {
const { path } = thread;

// Defensive: if a comment thread for this review thread id already exists in any of the
// thread maps (e.g. because doInitializeCommentThreads already created it from the cache
// before this event was processed), update it in place instead of creating a duplicate
// VS Code comment thread.
const existingMatch = this._findMatchingThread(thread);
if (existingMatch.index > -1) {
const matchingThread = existingMatch.threadMap[thread.path][existingMatch.index];
updateThread(this._context, matchingThread, thread, githubRepositories);
continue;
Comment thread
alexr00 marked this conversation as resolved.
Comment thread
alexr00 marked this conversation as resolved.
}

const index = await arrayFindIndexAsync(this._pendingCommentThreadAdds, async t => {
const fileName = this._folderRepoManager.gitRelativeRootPath(t.uri.path);
if (fileName !== thread.path) {
Expand Down Expand Up @@ -358,13 +369,16 @@ export class ReviewCommentController extends CommentControllerBase implements Co
let index = threadMap[thread.path]?.findIndex(t => t.gitHubThreadId === thread.id) ?? -1;
if ((index === -1) && thread.isOutdated) {
// The thread has become outdated and needs to be moved to the obsolete threads.
index = this._workspaceFileChangeCommentThreads[thread.path]?.findIndex(t => t.gitHubThreadId === thread.id) ?? -1;
if (index > -1) {
const matchingThread = this._workspaceFileChangeCommentThreads[thread.path]!.splice(index, 1)[0];
const workspaceIndex = this._workspaceFileChangeCommentThreads[thread.path]?.findIndex(t => t.gitHubThreadId === thread.id) ?? -1;
if (workspaceIndex > -1) {
const matchingThread = this._workspaceFileChangeCommentThreads[thread.path]!.splice(workspaceIndex, 1)[0];
if (!this._obsoleteFileChangeCommentThreads[thread.path]) {
this._obsoleteFileChangeCommentThreads[thread.path] = [];
}
this._obsoleteFileChangeCommentThreads[thread.path]!.push(matchingThread);
// `threadMap` already references `_obsoleteFileChangeCommentThreads`; the matching
// thread is now the last element of the obsolete array for this path.
index = this._obsoleteFileChangeCommentThreads[thread.path]!.length - 1;
}
}
return { threadMap, index };
Expand Down