Skip to content
Merged
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
139 changes: 90 additions & 49 deletions src/agent/acp/AcpConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,47 @@
// Track if child process was spawned with detached: true (needs process group kill)
private isDetached = false;

/**
* Kill the current child process (if any) and clear process-related state.
* Handles platform differences: Windows taskkill tree kill, POSIX detached
* process group kill, and standard SIGTERM.
*
* Used by both disconnect() and retry paths. Does NOT reset session-level
* state (sessionId, backend, etc.) — that is disconnect()'s responsibility.
*/
private async terminateChild(): Promise<void> {
if (!this.child) {
this.isDetached = false;
return;
}

const pid = this.child.pid;
if (process.platform === 'win32' && pid) {
// Windows: shell:true spawns cmd.exe as parent; taskkill /T kills the tree.
try {
await execFile('taskkill', ['/PID', String(pid), '/T'], { windowsHide: true, timeout: 2000 });
} catch {
try {
await execFile('taskkill', ['/PID', String(pid), '/T', '/F'], { windowsHide: true, timeout: 2000 });
} catch (forceError) {
console.warn(`[ACP] taskkill /F failed for PID ${pid}:`, forceError);
}
}
} else if (this.isDetached && pid) {
// POSIX detached: negative PID kills the entire process group (setsid).
try {
process.kill(-pid, 'SIGTERM');
} catch {
this.child.kill('SIGTERM');
}
} else {
this.child.kill('SIGTERM');
}

this.child = null;
this.isDetached = false;
}

/**
* Prepare a clean environment for npx-based ACP backends.
* Removes Node.js debugging vars and npm lifecycle vars that can interfere
Expand Down Expand Up @@ -251,11 +292,28 @@

this.ensureMinNodeVersion(cleanEnv, 20, 10, 'Claude ACP bridge');

// Resolve npx from the same bin directory as the verified node binary
// to avoid picking up a stale globally-installed npx (pre npm 7)
const isWindows = process.platform === 'win32';
const spawnCommand = resolveNpxPath(cleanEnv);
const spawnArgs = ['--prefer-offline', '@zed-industries/claude-agent-acp@0.18.0'];

// Phase 1: Try with --prefer-offline for fast startup (~1-2s)
try {
await this.spawnAndSetupClaude(spawnCommand, cleanEnv, workingDir, isWindows, true);
} catch (firstError) {
// Phase 2: Retry without --prefer-offline to refresh stale cache (~3-5s)
// This handles upgrades where cached registry metadata is outdated
console.warn('[ACP] --prefer-offline failed, retrying with fresh registry lookup:', firstError instanceof Error ? firstError.message : String(firstError));

// Terminate the first child (may still be running if initialize() timed out)
// to prevent orphaned processes and stale exit handlers interfering with retry
await this.terminateChild();
this.isSetupComplete = false;

await this.spawnAndSetupClaude(spawnCommand, cleanEnv, workingDir, isWindows, false);
}
}

private async spawnAndSetupClaude(spawnCommand: string, cleanEnv: Record<string, string | undefined>, workingDir: string, isWindows: boolean, preferOffline: boolean): Promise<void> {
const spawnArgs = ['--yes', ...(preferOffline ? ['--prefer-offline'] : []), '@zed-industries/claude-agent-acp@0.18.0'];

const spawnStart = Date.now();
this.child = spawn(spawnCommand, spawnArgs, {
Expand All @@ -264,7 +322,9 @@
env: cleanEnv,
shell: isWindows,
});
if (ACP_PERF_LOG) console.log(`[ACP-PERF] connect: claude process spawned ${Date.now() - spawnStart}ms`);
if (ACP_PERF_LOG) {
console.log(`[ACP-PERF] connect: claude process spawned ${Date.now() - spawnStart}ms (preferOffline=${preferOffline})`);
}

await this.setupChildProcessHandlers('claude');
}
Expand All @@ -279,22 +339,40 @@

this.ensureMinNodeVersion(cleanEnv, 20, 10, 'CodeBuddy ACP');

// Resolve npx from the verified node bin directory (same as connectClaude)
const isWindows = process.platform === 'win32';
const spawnCommand = resolveNpxPath(cleanEnv);
const spawnArgs = ['--yes', '--prefer-offline', '@tencent-ai/codebuddy-code', '--acp'];

// Load user's MCP config if available (~/.codebuddy/mcp.json)
// CodeBuddy CLI in --acp mode does not auto-load mcp.json, so we pass it explicitly
const mcpConfigPath = path.join(os.homedir(), '.codebuddy', 'mcp.json');
const extraArgs: string[] = [];
try {
await fs.access(mcpConfigPath);
spawnArgs.push('--mcp-config', mcpConfigPath);
extraArgs.push('--mcp-config', mcpConfigPath);
console.error(`[ACP] Loading CodeBuddy MCP config from ${mcpConfigPath}`);
} catch {
console.error('[ACP] No CodeBuddy MCP config found, starting without MCP servers');
}

// Phase 1: Try with --prefer-offline for fast startup
try {
await this.spawnAndSetupCodebuddy(spawnCommand, cleanEnv, workingDir, isWindows, extraArgs, true);
} catch (firstError) {
// Phase 2: Retry without --prefer-offline to refresh stale cache
console.warn('[ACP] CodeBuddy --prefer-offline failed, retrying with fresh registry lookup:', firstError instanceof Error ? firstError.message : String(firstError));

// Terminate the first child (may still be running if initialize() timed out)
// to prevent orphaned processes and stale exit handlers interfering with retry
await this.terminateChild();
this.isSetupComplete = false;

await this.spawnAndSetupCodebuddy(spawnCommand, cleanEnv, workingDir, isWindows, extraArgs, false);
}
}

private async spawnAndSetupCodebuddy(spawnCommand: string, cleanEnv: Record<string, string | undefined>, workingDir: string, isWindows: boolean, extraArgs: string[], preferOffline: boolean): Promise<void> {

Check warning on line 373 in src/agent/acp/AcpConnection.ts

View workflow job for this annotation

GitHub Actions / Code Quality

This line has a length of 208. Maximum allowed is 200
const spawnArgs = ['--yes', ...(preferOffline ? ['--prefer-offline'] : []), '@tencent-ai/codebuddy-code', '--acp', ...extraArgs];

if (ACP_PERF_LOG) console.log(`[ACP-PERF] codebuddy: spawning ${spawnCommand} ${spawnArgs.join(' ')}`);
const spawnStart = Date.now();
// Use detached: true to create a new session (setsid) so the child
Expand All @@ -314,7 +392,9 @@
if (this.isDetached) {
this.child.unref();
}
if (ACP_PERF_LOG) console.log(`[ACP-PERF] codebuddy: process spawned ${Date.now() - spawnStart}ms`);
if (ACP_PERF_LOG) {
console.log(`[ACP-PERF] codebuddy: process spawned ${Date.now() - spawnStart}ms (preferOffline=${preferOffline})`);
}

const handlerStart = Date.now();
await this.setupChildProcessHandlers('codebuddy');
Expand Down Expand Up @@ -969,52 +1049,13 @@
}

async disconnect(): Promise<void> {
if (this.child) {
const pid = this.child.pid;
if (process.platform === 'win32' && pid) {
// When shell:true is used on Windows, this.child usually points to
// cmd.exe while the actual ACP CLI runs as a descendant process.
// taskkill /T ensures the full process tree is terminated.
// Step 1: Graceful tree kill (no /F) — gives the CLI a chance to clean up.
// Step 2: Force kill if graceful termination failed.
// Using async execFile to avoid blocking the Electron main process.
try {
await execFile('taskkill', ['/PID', String(pid), '/T'], {
windowsHide: true,
timeout: 2000,
});
} catch {
try {
await execFile('taskkill', ['/PID', String(pid), '/T', '/F'], {
windowsHide: true,
timeout: 2000,
});
} catch (forceError) {
console.warn(`[ACP] taskkill /F failed for PID ${pid}:`, forceError);
}
}
} else if (this.isDetached && pid) {
// For detached processes (CodeBuddy on non-Windows), kill the entire
// process group so npx's child CLI also terminates.
// Negative PID = process group kill (POSIX setsid).
try {
process.kill(-pid, 'SIGTERM');
} catch {
// Fallback: process group kill failed (e.g., already exited)
this.child.kill('SIGTERM');
}
} else {
this.child.kill('SIGTERM');
}
this.child = null;
}
await this.terminateChild();

// Reset state
// Reset session-level state
this.pendingRequests.clear();
this.sessionId = null;
this.isInitialized = false;
this.isSetupComplete = false;
this.isDetached = false;
this.backend = null;
this.initializeResponse = null;
this.configOptions = null;
Expand Down
Loading