Skip to content

Commit 5730484

Browse files
authored
Fix async octokit calls to check for auth first
2 parents 9e27309 + 51a2ba8 commit 5730484

File tree

4 files changed

+82
-29
lines changed

4 files changed

+82
-29
lines changed

package.json

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2503,11 +2503,6 @@
25032503
"default": true,
25042504
"description": "%github.copilot.config.customInstructionsInSystemMessage%"
25052505
},
2506-
"github.copilot.chat.customAgents.showOrganizationAndEnterpriseAgents": {
2507-
"type": "boolean",
2508-
"default": true,
2509-
"description": "%github.copilot.config.customAgents.showOrganizationAndEnterpriseAgents%"
2510-
},
25112506
"github.copilot.chat.agent.currentEditorContext.enabled": {
25122507
"type": "boolean",
25132508
"default": true,
@@ -3440,6 +3435,14 @@
34403435
"electron-fetch",
34413436
"node-fetch"
34423437
]
3438+
},
3439+
"github.copilot.chat.customAgents.showOrganizationAndEnterpriseAgents": {
3440+
"type": "boolean",
3441+
"default": false,
3442+
"description": "%github.copilot.config.customAgents.showOrganizationAndEnterpriseAgents%",
3443+
"tags": [
3444+
"experimental"
3445+
]
34433446
}
34443447
}
34453448
},

src/extension/agents/vscode-node/organizationAndEnterpriseAgentProvider.ts

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,19 @@ import { Disposable } from '../../../util/vs/base/common/lifecycle';
1414

1515
const AgentFileExtension = '.agent.md';
1616

17+
class UserNotSignedInError extends Error {
18+
constructor() {
19+
super('User is not signed in');
20+
}
21+
}
22+
1723
export class OrganizationAndEnterpriseAgentProvider extends Disposable implements vscode.CustomAgentsProvider {
1824

1925
private readonly _onDidChangeCustomAgents = this._register(new vscode.EventEmitter<void>());
2026
readonly onDidChangeCustomAgents = this._onDidChangeCustomAgents.event;
2127

2228
private isFetching = false;
23-
private memoryCache: vscode.CustomAgentResource[] | undefined = undefined;
29+
private memoryCache: vscode.CustomAgentResource[] | undefined;
2430

2531
constructor(
2632
@IOctoKitService private readonly octoKitService: IOctoKitService,
@@ -29,31 +35,29 @@ export class OrganizationAndEnterpriseAgentProvider extends Disposable implement
2935
@IFileSystemService private readonly fileSystem: IFileSystemService,
3036
) {
3137
super();
38+
39+
// Trigger async fetch to update cache. Note: this provider is re-created each time
40+
// the user signs in, so this will re-fetch on sign-in. See logic in conversationFeature.ts.
41+
this.fetchAndUpdateCache().catch(error => {
42+
this.logService.error(`[OrganizationAndEnterpriseAgentProvider] Error in background fetch: ${error}`);
43+
});
3244
}
3345

3446
private getCacheDir(): vscode.Uri {
3547
return vscode.Uri.joinPath(this.extensionContext.globalStorageUri, 'githubAgentsCache');
3648
}
3749

3850
async provideCustomAgents(
39-
options: vscode.CustomAgentQueryOptions,
51+
_options: vscode.CustomAgentQueryOptions,
4052
_token: vscode.CancellationToken
4153
): Promise<vscode.CustomAgentResource[]> {
4254
try {
43-
// If we have successfully fetched and cached in memory, return from memory
4455
if (this.memoryCache !== undefined) {
4556
return this.memoryCache;
4657
}
4758

48-
// Read from file cache first
49-
const fileCachedAgents = await this.readFromCache();
50-
51-
// Trigger async fetch to update cache
52-
this.fetchAndUpdateCache(options).catch(error => {
53-
this.logService.error(`[OrganizationAndEnterpriseAgentProvider] Error in background fetch: ${error}`);
54-
});
55-
56-
return fileCachedAgents;
59+
// Return results from file cache
60+
return await this.readFromCache();
5761
} catch (error) {
5862
this.logService.error(`[OrganizationAndEnterpriseAgentProvider] Error in provideCustomAgents: ${error}`);
5963
return [];
@@ -110,9 +114,15 @@ export class OrganizationAndEnterpriseAgentProvider extends Disposable implement
110114
}
111115
}
112116

113-
private async fetchAndUpdateCache(
114-
options: vscode.CustomAgentQueryOptions
115-
): Promise<void> {
117+
private async runWithAuthCheck<T>(operation: () => Promise<T>): Promise<T> {
118+
const user = await this.octoKitService.getCurrentAuthedUser();
119+
if (!user) {
120+
throw new UserNotSignedInError();
121+
}
122+
return operation();
123+
}
124+
125+
private async fetchAndUpdateCache(): Promise<void> {
116126
// Prevent concurrent fetches
117127
if (this.isFetching) {
118128
this.logService.trace('[OrganizationAndEnterpriseAgentProvider] Fetch already in progress, skipping');
@@ -121,10 +131,16 @@ export class OrganizationAndEnterpriseAgentProvider extends Disposable implement
121131

122132
this.isFetching = true;
123133
try {
134+
const user = await this.octoKitService.getCurrentAuthedUser();
135+
if (!user) {
136+
this.logService.trace('[OrganizationAndEnterpriseAgentProvider] User not signed in, skipping fetch');
137+
return;
138+
}
139+
124140
this.logService.trace('[OrganizationAndEnterpriseAgentProvider] Fetching custom agents from all user organizations');
125141

126142
// Get all organizations the user belongs to
127-
const organizations = await this.octoKitService.getUserOrganizations();
143+
const organizations = await this.runWithAuthCheck(() => this.octoKitService.getUserOrganizations());
128144
if (organizations.length === 0) {
129145
this.logService.trace('[OrganizationAndEnterpriseAgentProvider] User does not belong to any organizations');
130146
return;
@@ -133,9 +149,9 @@ export class OrganizationAndEnterpriseAgentProvider extends Disposable implement
133149
this.logService.trace(`[OrganizationAndEnterpriseAgentProvider] Found ${organizations.length} organizations: ${organizations.join(', ')}`);
134150

135151
// Convert VS Code API options to internal options
136-
const internalOptions = options ? {
152+
const internalOptions = {
137153
includeSources: ['org', 'enterprise'] // don't include 'repo'
138-
} satisfies CustomAgentListOptions : undefined;
154+
} satisfies CustomAgentListOptions;
139155

140156
// Fetch agents from all organizations
141157
const agentsByOrg = new Map<string, Map<string, CustomAgentListItem>>();
@@ -148,19 +164,23 @@ export class OrganizationAndEnterpriseAgentProvider extends Disposable implement
148164

149165
// Get the first repository for this organization to use in the API call
150166
// We can't just use .github-private because user may not have access to it
151-
const repos = await this.octoKitService.getOrganizationRepositories(org);
167+
const repos = await this.runWithAuthCheck(() => this.octoKitService.getOrganizationRepositories(org));
152168
if (repos.length === 0) {
153169
this.logService.trace(`[OrganizationAndEnterpriseAgentProvider] No repositories found for ${org}, skipping`);
154170
continue;
155171
}
156172

157173
const repoName = repos[0];
158-
const agents = await this.octoKitService.getCustomAgents(org, repoName, internalOptions);
174+
const agents = await this.runWithAuthCheck(() => this.octoKitService.getCustomAgents(org, repoName, internalOptions));
159175
for (const agent of agents) {
160176
agentsForOrg.set(agent.name, agent);
161177
}
162178
this.logService.trace(`[OrganizationAndEnterpriseAgentProvider] Fetched ${agents.length} agents from ${org} using repo ${repoName}`);
163179
} catch (error) {
180+
if (error instanceof UserNotSignedInError) {
181+
this.logService.trace('[OrganizationAndEnterpriseAgentProvider] User signed out during fetch, aborting');
182+
return;
183+
}
164184
this.logService.error(`[OrganizationAndEnterpriseAgentProvider] Error fetching agents from ${org}: ${error}`);
165185
hadAnyFetchErrors = true;
166186
}
@@ -222,12 +242,12 @@ export class OrganizationAndEnterpriseAgentProvider extends Disposable implement
222242
const filename = this.sanitizeFilename(agent.name) + AgentFileExtension;
223243

224244
// Fetch full agent details including prompt content
225-
const agentDetails = await this.octoKitService.getCustomAgentDetails(
245+
const agentDetails = await this.runWithAuthCheck(() => this.octoKitService.getCustomAgentDetails(
226246
agent.repo_owner,
227247
agent.repo_name,
228248
agent.name,
229249
agent.version
230-
);
250+
));
231251

232252
// Generate agent markdown file content
233253
if (agentDetails) {
@@ -236,6 +256,10 @@ export class OrganizationAndEnterpriseAgentProvider extends Disposable implement
236256
totalAgents++;
237257
}
238258
} catch (error) {
259+
if (error instanceof UserNotSignedInError) {
260+
this.logService.trace('[OrganizationAndEnterpriseAgentProvider] User signed out during fetch, aborting');
261+
return;
262+
}
239263
this.logService.error(`[OrganizationAndEnterpriseAgentProvider] Error fetching details for agent ${agent.name} from ${org}: ${error}`);
240264
hadFetchError = true;
241265
}

src/extension/agents/vscode-node/test/organizationAndEnterpriseAgentProvider.spec.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ suite('OrganizationAndEnterpriseAgentProvider', () => {
115115
mockOctoKitService,
116116
accessor.get(ILogService),
117117
mockExtensionContext as any,
118-
mockFileSystem
118+
mockFileSystem,
119119
);
120120
disposables.add(provider);
121121
return provider;
@@ -873,4 +873,30 @@ Test prompt
873873

874874
assert.equal(content, expectedContent);
875875
});
876+
877+
test('aborts fetch if user signs out during process', async () => {
878+
const provider = createProvider();
879+
880+
// Setup multiple organizations to ensure we have multiple steps
881+
mockOctoKitService.setUserOrganizations(['org1', 'org2']);
882+
mockOctoKitService.getOrganizationRepositories = async (org) => ['repo'];
883+
884+
// Mock getCustomAgents to simulate sign out after first org
885+
let callCount = 0;
886+
const originalGetCustomAgents = mockOctoKitService.getCustomAgents;
887+
mockOctoKitService.getCustomAgents = async (owner, repo, options) => {
888+
callCount++;
889+
if (callCount === 1) {
890+
// Sign out user after first call
891+
mockOctoKitService.getCurrentAuthedUser = async () => undefined as any;
892+
}
893+
return originalGetCustomAgents.call(mockOctoKitService, owner, repo, options);
894+
};
895+
896+
await provider.provideCustomAgents({}, {} as any);
897+
await new Promise(resolve => setTimeout(resolve, 100));
898+
899+
// Should have aborted after first org, so second org shouldn't be processed
900+
assert.equal(callCount, 1);
901+
});
876902
});

src/platform/configuration/common/configurationService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -867,7 +867,7 @@ export namespace ConfigKey {
867867
export const EnableAlternateGptPrompt = defineSetting<boolean>('chat.alternateGptPrompt.enabled', ConfigType.ExperimentBased, false);
868868

869869
/** Enable custom agents from GitHub Enterprise/Organizations */
870-
export const ShowOrganizationAndEnterpriseAgents = defineSetting<boolean>('chat.customAgents.showOrganizationAndEnterpriseAgents', ConfigType.Simple, true);
870+
export const ShowOrganizationAndEnterpriseAgents = defineSetting<boolean>('chat.customAgents.showOrganizationAndEnterpriseAgents', ConfigType.Simple, false);
871871

872872
export const CompletionsFetcher = defineSetting<FetcherId | undefined>('chat.completionsFetcher', ConfigType.ExperimentBased, undefined);
873873
export const NextEditSuggestionsFetcher = defineSetting<FetcherId | undefined>('chat.nesFetcher', ConfigType.ExperimentBased, undefined);

0 commit comments

Comments
 (0)