fix(acp): resolve test failures and ensure proper session initialization

This commit is contained in:
Bryan Morgan
2026-02-01 00:18:45 -05:00
parent 1f121383bb
commit da9bccf94d

View File

@@ -234,54 +234,30 @@ export class GeminiAgent {
cwd: string,
mcpServers: acp.McpServer[],
): Promise<Config> {
// Authenticate BEFORE initializing configuration or starting MCP servers
let isAuthenticated = false;
// Check if we have a selected auth type in global settings (loaded in constructor)
if (this.settings.merged.security.auth.selectedType) {
try {
// We need a temporary config just to refresh auth, but we MUST NOT
// start MCP servers or do anything heavy yet.
// However, refreshAuth is a method on Config.
// Ideally, we should check auth status independently or use a lightweight config.
// For now, we will create the config BUT we must trust the `refreshAuth` check
// to block before we use the config for anything sensitive.
// WAIT - newSessionConfig STARTS MCP servers. We cannot call it yet.
// Refactor: We need to check auth using the *existing* settings and a dummy config
// or just the auth service if possible.
// But `config.refreshAuth` is the standard way.
// If `newSessionConfig` starts MCP servers, we are in a bind.
// Let's look at `newSessionConfig`. It calls `loadCliConfig` which might start things.
// Actually `loadCliConfig` just loads config. `config.initialize()` starts things?
// `newSessionConfig` calls `config.initialize()`.
// Correct approach:
// 1. Create config WITHOUT initializing it (no MCP servers started).
// 2. Check Auth.
// 3. Initialize Config (start MCP servers).
// However, `newSessionConfig` does both. We need to split it.
// Or we can just do the auth check using the global `this.settings`?
// `this.config.refreshAuth` uses the *current* config's auth service.
// We can use `this.config` (the agent's main config) to check auth?
// The agent has a `config` passed in constructor.
await this.config.refreshAuth(
this.settings.merged.security.auth.selectedType,
);
isAuthenticated = true;
} catch (e) {
debugLogger.error(`Authentication failed: ${e}`);
}
}
if (!isAuthenticated) {
const selectedAuthType = this.settings.merged.security.auth.selectedType;
if (!selectedAuthType) {
throw acp.RequestError.authRequired();
}
// Now that we are authenticated, it is safe to load the session config
// and start MCP servers (which might be malicious, but the user is authenticated).
return this.newSessionConfig(sessionId, cwd, mcpServers);
// 1. Create config WITHOUT initializing it (no MCP servers started yet)
const config = await this.newSessionConfig(sessionId, cwd, mcpServers);
// 2. Authenticate BEFORE initializing configuration or starting MCP servers.
// This satisfies the security requirement to verify the user before executing
// potentially unsafe server definitions.
try {
await config.refreshAuth(selectedAuthType);
} catch (e) {
debugLogger.error(`Authentication failed: ${e}`);
throw acp.RequestError.authRequired();
}
// 3. Now that we are authenticated, it is safe to initialize the config
// which starts the MCP servers and other heavy resources.
await config.initialize();
startupProfiler.flush(config);
return config;
}
async newSessionConfig(
@@ -328,8 +304,6 @@ export class GeminiAgent {
const config = await loadCliConfig(settings, sessionId, this.argv, { cwd });
await config.initialize();
startupProfiler.flush(config);
return config;
}