fix(vscode-ide-companion): improve ACP connection and session management

Enhance session/load and session/list ACP methods with proper cwd handling and pagination support

- Add workingDir tracking in AcpConnection

- Improve parameter handling in loadSession and listSessions

- Add pagination support for session listing

- Fix null/undefined checks in message handling
This commit is contained in:
yiliang114
2025-12-06 21:45:18 +08:00
parent 413c143004
commit e538a3d1bf
3 changed files with 35 additions and 27 deletions

View File

@@ -26,29 +26,15 @@ import { determineNodePathForCli } from '../cli/cliPathDetector.js';
* ACP Connection Handler for VSCode Extension * ACP Connection Handler for VSCode Extension
* *
* This class implements the client side of the ACP (Agent Communication Protocol). * This class implements the client side of the ACP (Agent Communication Protocol).
*
* Implementation Status:
*
* Client Methods (Methods this class implements, called by CLI):
* ✅ session/update - Handle session updates via onSessionUpdate callback
* ✅ session/request_permission - Request user permission for tool execution
* ✅ fs/read_text_file - Read file from workspace
* ✅ fs/write_text_file - Write file to workspace
*
* Agent Methods (Methods CLI implements, called by this class):
* ✅ initialize - Initialize ACP protocol connection
* ✅ authenticate - Authenticate with selected auth method
* ✅ session/new - Create new chat session
* ✅ session/prompt - Send user message to agent
* ✅ session/cancel - Cancel current generation
* ✅ session/load - Load previous session
* ✅ session/save - Save current session
*/ */
export class AcpConnection { export class AcpConnection {
private child: ChildProcess | null = null; private child: ChildProcess | null = null;
private pendingRequests = new Map<number, PendingRequest<unknown>>(); private pendingRequests = new Map<number, PendingRequest<unknown>>();
private nextRequestId = { value: 0 }; private nextRequestId = { value: 0 };
private backend: AcpBackend | null = null; private backend: AcpBackend | null = null;
// Remember the working dir provided at connect() so later ACP calls
// that require cwd (e.g. session/list) can include it.
private workingDir: string = process.cwd();
private messageHandler: AcpMessageHandler; private messageHandler: AcpMessageHandler;
private sessionManager: AcpSessionManager; private sessionManager: AcpSessionManager;
@@ -83,6 +69,7 @@ export class AcpConnection {
} }
this.backend = backend; this.backend = backend;
this.workingDir = workingDir;
const isWindows = process.platform === 'win32'; const isWindows = process.platform === 'win32';
const env = { ...process.env }; const env = { ...process.env };
@@ -327,12 +314,16 @@ export class AcpConnection {
* @param sessionId - Session ID * @param sessionId - Session ID
* @returns Load response * @returns Load response
*/ */
async loadSession(sessionId: string): Promise<AcpResponse> { async loadSession(
sessionId: string,
cwdOverride?: string,
): Promise<AcpResponse> {
return this.sessionManager.loadSession( return this.sessionManager.loadSession(
sessionId, sessionId,
this.child, this.child,
this.pendingRequests, this.pendingRequests,
this.nextRequestId, this.nextRequestId,
cwdOverride || this.workingDir,
); );
} }
@@ -341,11 +332,16 @@ export class AcpConnection {
* *
* @returns Session list response * @returns Session list response
*/ */
async listSessions(): Promise<AcpResponse> { async listSessions(options?: {
cursor?: number;
size?: number;
}): Promise<AcpResponse> {
return this.sessionManager.listSessions( return this.sessionManager.listSessions(
this.child, this.child,
this.pendingRequests, this.pendingRequests,
this.nextRequestId, this.nextRequestId,
this.workingDir,
options,
); );
} }

View File

@@ -107,7 +107,8 @@ export class AcpMessageHandler {
if ('result' in message) { if ('result' in message) {
console.log( console.log(
`[ACP] Response for ${method}:`, `[ACP] Response for ${method}:`,
JSON.stringify(message.result).substring(0, 200), // JSON.stringify(message.result).substring(0, 200),
message.result,
); );
if ( if (
message.result && message.result &&
@@ -204,11 +205,11 @@ export class AcpMessageHandler {
}> { }> {
try { try {
const response = await callbacks.onPermissionRequest(params); const response = await callbacks.onPermissionRequest(params);
const optionId = response.optionId; const optionId = response?.optionId;
console.log('[ACP] Permission request:', optionId); console.log('[ACP] Permission request:', optionId);
// Handle cancel, deny, or allow // Handle cancel, deny, or allow
let outcome: string; let outcome: string;
if (optionId.includes('reject') || optionId === 'cancel') { if (optionId && (optionId.includes('reject') || optionId === 'cancel')) {
outcome = 'cancelled'; outcome = 'cancelled';
} else { } else {
outcome = 'selected'; outcome = 'selected';

View File

@@ -196,7 +196,7 @@ export class AcpSessionManager {
nextRequestId, nextRequestId,
); );
this.sessionId = response.sessionId || null; this.sessionId = (response && response.sessionId) || null;
console.log('[ACP] Session created with ID:', this.sessionId); console.log('[ACP] Session created with ID:', this.sessionId);
return response; return response;
} }
@@ -247,11 +247,12 @@ export class AcpSessionManager {
child: ChildProcess | null, child: ChildProcess | null,
pendingRequests: Map<number, PendingRequest<unknown>>, pendingRequests: Map<number, PendingRequest<unknown>>,
nextRequestId: { value: number }, nextRequestId: { value: number },
cwd: string = process.cwd(),
): Promise<AcpResponse> { ): Promise<AcpResponse> {
console.log('[ACP] Sending session/load request for session:', sessionId); console.log('[ACP] Sending session/load request for session:', sessionId);
console.log('[ACP] Request parameters:', { console.log('[ACP] Request parameters:', {
sessionId, sessionId,
cwd: process.cwd(), cwd,
mcpServers: [], mcpServers: [],
}); });
@@ -260,7 +261,7 @@ export class AcpSessionManager {
AGENT_METHODS.session_load, AGENT_METHODS.session_load,
{ {
sessionId, sessionId,
cwd: process.cwd(), cwd,
mcpServers: [], mcpServers: [],
}, },
child, child,
@@ -274,10 +275,13 @@ export class AcpSessionManager {
); );
// Check if response contains an error // Check if response contains an error
if (response.error) { if (response && response.error) {
console.error('[ACP] Session load returned error:', response.error); console.error('[ACP] Session load returned error:', response.error);
} else { } else {
console.log('[ACP] Session load succeeded'); console.log('[ACP] Session load succeeded');
// session/load returns null on success per schema; update local sessionId
// so subsequent prompts use the loaded session.
this.sessionId = sessionId;
} }
return response; return response;
@@ -302,12 +306,19 @@ export class AcpSessionManager {
child: ChildProcess | null, child: ChildProcess | null,
pendingRequests: Map<number, PendingRequest<unknown>>, pendingRequests: Map<number, PendingRequest<unknown>>,
nextRequestId: { value: number }, nextRequestId: { value: number },
cwd: string = process.cwd(),
options?: { cursor?: number; size?: number },
): Promise<AcpResponse> { ): Promise<AcpResponse> {
console.log('[ACP] Requesting session list...'); console.log('[ACP] Requesting session list...');
try { try {
// session/list requires cwd in params per ACP schema
const params: Record<string, unknown> = { cwd };
if (options?.cursor !== undefined) params.cursor = options.cursor;
if (options?.size !== undefined) params.size = options.size;
const response = await this.sendRequest<AcpResponse>( const response = await this.sendRequest<AcpResponse>(
AGENT_METHODS.session_list, AGENT_METHODS.session_list,
{}, params,
child, child,
pendingRequests, pendingRequests,
nextRequestId, nextRequestId,