From 5431f941f6cca22fcdd51b9acd09a2cc66f86458 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:16:34 +0000 Subject: [PATCH 1/3] Initial plan From f1d810d85d58c4bac219934d75cfad9a7d64c149 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:55:15 +0000 Subject: [PATCH 2/3] Fix race condition in issues tree view where getIssues errors prevent tree refresh The issues tree view could get permanently stuck empty because: 1. `setIssues()` used the `new Promise(async executor)` anti-pattern and only fired `_onDidChangeIssueData` on success. If `getIssues()` threw (e.g. due to a transient network error), the promise rejected silently, the event never fired, and the tree never learned to refresh. 2. `setIssueData()` didn't fire `_onDidChangeIssueData` after processing all queries, so if all queries failed, the tree stayed permanently empty until a git commit triggered a new `setIssueData` call. Fix: - Refactor `setIssues()` to use async/await properly, catch errors gracefully (resolving with undefined instead of rejecting), and fire the change event in a `finally` block so it always fires. - Fire `_onDidChangeIssueData` at the end of `setIssueData()` to guarantee the tree always gets a refresh signal after data loading completes. Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/42a1cce0-564c-42dd-afc9-909f2730d385 Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> --- src/issues/stateManager.ts | 22 +++-- src/test/issues/stateManager.test.ts | 136 +++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 10 deletions(-) diff --git a/src/issues/stateManager.ts b/src/issues/stateManager.ts index 14316c0818..d9d2eb680d 100644 --- a/src/issues/stateManager.ts +++ b/src/issues/stateManager.ts @@ -345,20 +345,22 @@ export class StateManager { singleRepoState.maxIssueNumber = await folderManager.getMaxIssue(); singleRepoState.lastHead = folderManager.repository.state.HEAD?.commit; singleRepoState.lastBranch = folderManager.repository.state.HEAD?.name; + this._onDidChangeIssueData.fire(); } - private setIssues(folderManager: FolderRepositoryManager, query: string): Promise { - return new Promise(async resolve => { + private async setIssues(folderManager: FolderRepositoryManager, query: string): Promise { + try { const issues = await folderManager.getIssues(query, { fetchNextPage: false, fetchOnePagePerRepo: true }); + return issues?.items.map(item => { + const issueItem: IssueItem = item as IssueItem; + issueItem.uri = folderManager.repository.rootUri; + return issueItem; + }); + } catch (e) { + return undefined; + } finally { this._onDidChangeIssueData.fire(); - resolve( - issues?.items.map(item => { - const issueItem: IssueItem = item as IssueItem; - issueItem.uri = folderManager.repository.rootUri; - return issueItem; - }), - ); - }); + } } private async setCurrentIssueFromBranch(singleRepoState: SingleRepoState, branchName: string, silent: boolean = false) { diff --git a/src/test/issues/stateManager.test.ts b/src/test/issues/stateManager.test.ts index 4f2549ac0f..f2984511af 100644 --- a/src/test/issues/stateManager.test.ts +++ b/src/test/issues/stateManager.test.ts @@ -169,4 +169,140 @@ describe('StateManager branch behavior with useBranchForIssues setting', functio vscode.workspace.getConfiguration = originalGetConfiguration; } }); + + it('should fire onDidChangeIssueData even when getIssues throws', async function () { + const mockUri = vscode.Uri.parse('file:///test'); + const mockFolderManager = { + repository: { rootUri: mockUri, state: { HEAD: { commit: 'abc123' }, remotes: [] } }, + getIssues: async () => { + throw new Error('Network error'); + }, + getMaxIssue: async () => 0, + }; + + const originalGetConfiguration = vscode.workspace.getConfiguration; + vscode.workspace.getConfiguration = (section?: string) => { + if (section === ISSUES_SETTINGS_NAMESPACE) { + return { + get: (key: string, defaultValue?: any) => { + if (key === 'queries') { + return [{ label: 'Test', query: 'is:open assignee:@me repo:owner/repo', groupBy: [] }]; + } + return defaultValue; + }, + } as any; + } + return originalGetConfiguration(section); + }; + + try { + const sm = new StateManager(undefined as any, { + folderManagers: [mockFolderManager], + credentialStore: { isAnyAuthenticated: () => true, getCurrentUser: async () => ({ login: 'testuser' }) }, + } as any, mockContext); + + let changeEventCount = 0; + sm.onDidChangeIssueData(() => changeEventCount++); + + await (sm as any).setIssueData(mockFolderManager); + + // The event should have fired even though getIssues threw + assert.ok(changeEventCount > 0, 'onDidChangeIssueData should fire even when getIssues fails'); + + // The promise in the collection should resolve to undefined issues (not reject) + const collection = sm.getIssueCollection(mockUri); + const queryResult = await collection.get('Test'); + assert.strictEqual(queryResult?.issues, undefined, 'Issues should be undefined when getIssues fails'); + } finally { + vscode.workspace.getConfiguration = originalGetConfiguration; + } + }); + + it('should fire onDidChangeIssueData after setIssueData completes', async function () { + const mockUri = vscode.Uri.parse('file:///test'); + const mockFolderManager = { + repository: { rootUri: mockUri, state: { HEAD: { commit: 'abc123' }, remotes: [] } }, + getIssues: async () => { + return { items: [], hasMorePages: false, hasUnsearchedRepositories: false, totalCount: 0 }; + }, + getMaxIssue: async () => 0, + }; + + const originalGetConfiguration = vscode.workspace.getConfiguration; + vscode.workspace.getConfiguration = (section?: string) => { + if (section === ISSUES_SETTINGS_NAMESPACE) { + return { + get: (key: string, defaultValue?: any) => { + if (key === 'queries') { + return [{ label: 'Test', query: 'is:open assignee:@me repo:owner/repo', groupBy: [] }]; + } + return defaultValue; + }, + } as any; + } + return originalGetConfiguration(section); + }; + + try { + const sm = new StateManager(undefined as any, { + folderManagers: [mockFolderManager], + credentialStore: { isAnyAuthenticated: () => true, getCurrentUser: async () => ({ login: 'testuser' }) }, + } as any, mockContext); + + let changeEventCount = 0; + sm.onDidChangeIssueData(() => changeEventCount++); + + await (sm as any).setIssueData(mockFolderManager); + + // The event should fire at least twice: once from setIssues (per-query) and once from setIssueData (final) + assert.ok(changeEventCount >= 2, `onDidChangeIssueData should fire at least twice, but fired ${changeEventCount} times`); + } finally { + vscode.workspace.getConfiguration = originalGetConfiguration; + } + }); + + it('should not reject promises in issueCollection when getIssues throws', async function () { + const mockUri = vscode.Uri.parse('file:///test'); + const mockFolderManager = { + repository: { rootUri: mockUri, state: { HEAD: { commit: 'abc123' }, remotes: [] } }, + getIssues: async () => { + throw new Error('API error'); + }, + getMaxIssue: async () => 0, + }; + + const originalGetConfiguration = vscode.workspace.getConfiguration; + vscode.workspace.getConfiguration = (section?: string) => { + if (section === ISSUES_SETTINGS_NAMESPACE) { + return { + get: (key: string, defaultValue?: any) => { + if (key === 'queries') { + return [{ label: 'Test', query: 'is:open repo:owner/repo', groupBy: [] }]; + } + return defaultValue; + }, + } as any; + } + return originalGetConfiguration(section); + }; + + try { + const sm = new StateManager(undefined as any, { + folderManagers: [mockFolderManager], + credentialStore: { isAnyAuthenticated: () => true, getCurrentUser: async () => ({ login: 'testuser' }) }, + } as any, mockContext); + + await (sm as any).setIssueData(mockFolderManager); + + // Verify that the promises in issueCollection resolve (not reject) + const collection = sm.getIssueCollection(mockUri); + for (const [, promise] of collection) { + const result = await promise; + assert.ok(result !== undefined, 'Promise should resolve, not reject'); + assert.strictEqual(result.issues, undefined, 'Issues should be undefined on error'); + } + } finally { + vscode.workspace.getConfiguration = originalGetConfiguration; + } + }); }); \ No newline at end of file From 2fd53fc890de13a5687759891caf4ca149ee1581 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:58:51 +0000 Subject: [PATCH 3/3] Address review feedback: add comment to catch block, tighten test assertion Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/42a1cce0-564c-42dd-afc9-909f2730d385 Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> --- src/issues/stateManager.ts | 4 +++- src/test/issues/stateManager.test.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/issues/stateManager.ts b/src/issues/stateManager.ts index d9d2eb680d..d9d2b74ed1 100644 --- a/src/issues/stateManager.ts +++ b/src/issues/stateManager.ts @@ -356,7 +356,9 @@ export class StateManager { issueItem.uri = folderManager.repository.rootUri; return issueItem; }); - } catch (e) { + } catch { + // Errors from fetching issues are expected (e.g. network failures). + // Return undefined so the tree shows an empty state for this query. return undefined; } finally { this._onDidChangeIssueData.fire(); diff --git a/src/test/issues/stateManager.test.ts b/src/test/issues/stateManager.test.ts index f2984511af..64ad1ed274 100644 --- a/src/test/issues/stateManager.test.ts +++ b/src/test/issues/stateManager.test.ts @@ -255,7 +255,7 @@ describe('StateManager branch behavior with useBranchForIssues setting', functio await (sm as any).setIssueData(mockFolderManager); // The event should fire at least twice: once from setIssues (per-query) and once from setIssueData (final) - assert.ok(changeEventCount >= 2, `onDidChangeIssueData should fire at least twice, but fired ${changeEventCount} times`); + assert.strictEqual(changeEventCount, 2, `onDidChangeIssueData should fire exactly twice for a single query, but fired ${changeEventCount} times`); } finally { vscode.workspace.getConfiguration = originalGetConfiguration; }