Skip to content
Merged
Show file tree
Hide file tree
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
204 changes: 204 additions & 0 deletions packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ vi.mock('@qwen-code/qwen-code-core', () => ({
SessionStartSource: {
Startup: 'startup',
Resume: 'resume',
Branch: 'branch',
Clear: 'clear',
Compact: 'compact',
},
SessionEndReason: {
PromptInputExit: 'prompt_input_exit',
Expand Down Expand Up @@ -851,6 +854,207 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
await agentPromise;
});

it('bootstraps ACP config without initializing Gemini chat', async () => {
await setupSessionMocks('session-bootstrap-skip');

const agentPromise = runAcpAgent(
mockConfig,
makeSessionSettings(),
mockArgv,
);
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());

expect(mockConfig.initialize).toHaveBeenCalledWith({
skipGeminiInitialization: true,
});

mockConnectionState.resolve();
await agentPromise;
});

it('first ACP session fires SessionStart only from the real session initialize path', async () => {
const innerConfig = await setupSessionMocks(
'session-no-direct-session-start',
);
const fireSessionStartEvent = vi.fn().mockResolvedValue(undefined);
const initialize = vi.fn().mockImplementation(async () => {
await fireSessionStartEvent('startup', 'test-model', 'default');
});
innerConfig.getHookSystem = vi.fn().mockReturnValue({
fireSessionStartEvent,
});
innerConfig.getDisableAllHooks = vi.fn().mockReturnValue(false);
innerConfig.hasHooksForEvent = vi.fn().mockReturnValue(true);
innerConfig.getModel = vi.fn().mockReturnValue('test-model');
innerConfig.getApprovalMode = vi.fn().mockReturnValue('default');
innerConfig.getGeminiClient = vi.fn().mockReturnValue({
isInitialized: vi.fn().mockReturnValue(false),
initialize,
});

const agentPromise = runAcpAgent(
mockConfig,
makeSessionSettings(),
mockArgv,
);
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());

const agent = capturedAgentFactory!({
get closed() {
return mockConnectionState.promise;
},
}) as AgentLike;

await agent.newSession({ cwd: '/tmp', mcpServers: [] });

expect(mockConfig.initialize).toHaveBeenCalledWith({
skipGeminiInitialization: true,
});
expect(initialize).toHaveBeenCalledTimes(1);
expect(fireSessionStartEvent).toHaveBeenCalledTimes(1);
expect(fireSessionStartEvent).toHaveBeenCalledWith(
'startup',
'test-model',
'default',
);

mockConnectionState.resolve();
await agentPromise;
});

it('does not directly re-fire SessionStart for subsequent ACP sessions when GeminiClient is already initialized', async () => {
const innerConfig = await setupSessionMocks(
'session-followup-session-start',
);
const fireSessionStartEvent = vi.fn().mockResolvedValue(undefined);
const initialize = vi.fn().mockResolvedValue(undefined);
innerConfig.getHookSystem = vi.fn().mockReturnValue({
fireSessionStartEvent,
});
innerConfig.getDisableAllHooks = vi.fn().mockReturnValue(false);
innerConfig.hasHooksForEvent = vi.fn().mockReturnValue(true);
innerConfig.getModel = vi.fn().mockReturnValue('test-model');
innerConfig.getApprovalMode = vi.fn().mockReturnValue('default');
innerConfig.getGeminiClient = vi
.fn()
.mockReturnValueOnce({
isInitialized: vi.fn().mockReturnValue(false),
initialize,
})
.mockReturnValueOnce({
isInitialized: vi.fn().mockReturnValue(true),
initialize,
});

const agentPromise = runAcpAgent(
mockConfig,
makeSessionSettings(),
mockArgv,
);
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());

const agent = capturedAgentFactory!({
get closed() {
return mockConnectionState.promise;
},
}) as AgentLike;

await agent.newSession({ cwd: '/tmp', mcpServers: [] });
await agent.newSession({ cwd: '/tmp', mcpServers: [] });

expect(initialize).toHaveBeenCalledTimes(1);
expect(fireSessionStartEvent).not.toHaveBeenCalled();

mockConnectionState.resolve();
await agentPromise;
});

it('fires SessionEnd for each active ACP session config on connection.closed', async () => {
const bootstrapHookSystem = {
fireSessionEndEvent: vi.fn().mockResolvedValue(undefined),
fireSessionStartEvent: vi.fn().mockResolvedValue(undefined),
};
mockConfig.getHookSystem = vi.fn().mockReturnValue(bootstrapHookSystem);
mockConfig.hasHooksForEvent = vi
.fn()
.mockImplementation((event: string) => event === 'SessionEnd');

const innerConfigA = await setupSessionMocks('session-end-a');
const sessionHookSystemA = {
fireSessionEndEvent: vi.fn().mockResolvedValue(undefined),
fireSessionStartEvent: vi.fn().mockResolvedValue(undefined),
};
innerConfigA.getHookSystem = vi.fn().mockReturnValue(sessionHookSystemA);
innerConfigA.getDisableAllHooks = vi.fn().mockReturnValue(false);
innerConfigA.hasHooksForEvent = vi
.fn()
.mockImplementation((event: string) => event === 'SessionEnd');
innerConfigA.getGeminiClient = vi.fn().mockReturnValue({
isInitialized: vi.fn().mockReturnValue(false),
initialize: vi.fn().mockResolvedValue(undefined),
});

const innerConfigB = makeInnerConfig();
innerConfigB.getSessionId = vi.fn().mockReturnValue('session-end-b');
const sessionHookSystemB = {
fireSessionEndEvent: vi.fn().mockResolvedValue(undefined),
fireSessionStartEvent: vi.fn().mockResolvedValue(undefined),
};
innerConfigB.getHookSystem = vi.fn().mockReturnValue(sessionHookSystemB);
innerConfigB.getDisableAllHooks = vi.fn().mockReturnValue(false);
innerConfigB.hasHooksForEvent = vi
.fn()
.mockImplementation((event: string) => event === 'SessionEnd');
innerConfigB.getGeminiClient = vi.fn().mockReturnValue({
isInitialized: vi.fn().mockReturnValue(false),
initialize: vi.fn().mockResolvedValue(undefined),
});
vi.mocked(loadCliConfig)
.mockResolvedValueOnce(innerConfigA as unknown as Config)
.mockResolvedValueOnce(innerConfigB as unknown as Config);
vi.mocked(Session).mockImplementation((...args: unknown[]) => {
const sessionId = args[0] as string;
const cfg = sessionId === 'session-end-a' ? innerConfigA : innerConfigB;
return {
getId: vi.fn().mockReturnValue(sessionId),
getConfig: vi.fn().mockReturnValue(cfg),
sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined),
replayHistory: vi.fn().mockResolvedValue(undefined),
installRewriter: vi.fn(),
} as unknown as InstanceType<typeof Session>;
});
vi.mocked(loadSettings).mockReturnValue(makeSessionSettings());

const agentPromise = runAcpAgent(
mockConfig,
makeSessionSettings(),
mockArgv,
);
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());

const agent = capturedAgentFactory!({
get closed() {
return mockConnectionState.promise;
},
}) as AgentLike;

await agent.newSession({ cwd: '/tmp', mcpServers: [] });
await agent.newSession({ cwd: '/tmp', mcpServers: [] });

mockConnectionState.resolve();
await agentPromise;

expect(bootstrapHookSystem.fireSessionEndEvent).toHaveBeenCalledWith(
SessionEndReason.PromptInputExit,
);
expect(sessionHookSystemA.fireSessionEndEvent).toHaveBeenCalledWith(
SessionEndReason.PromptInputExit,
);
expect(sessionHookSystemB.fireSessionEndEvent).toHaveBeenCalledWith(
SessionEndReason.PromptInputExit,
);
});

it('rewindSession extension method rewinds the active session', async () => {
const sessionId = '11111111-1111-1111-1111-111111111111';
await setupSessionMocks(sessionId);
Expand Down
68 changes: 37 additions & 31 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ import {
type Config,
type ConversationRecord,
type DeviceAuthorizationData,
SessionStartSource,
SessionEndReason,
type PermissionMode,
} from '@qwen-code/qwen-code-core';
import {
AgentSideConnection,
Expand Down Expand Up @@ -81,9 +79,10 @@ export async function runAcpAgent(
settings: LoadedSettings,
argv: CliArgs,
) {
// Initialize config to set up hookSystem (required for SessionStart/SessionEnd hooks)
// This is needed because gemini.tsx calls runAcpAgent without calling config.initialize()
await config.initialize();
// Initialize config to set up ACP bootstrap services (hooks, tools, MCP)
// without creating a chat session. The real per-session Config will own
// GeminiClient.initialize() and any SessionStart hook execution.
await config.initialize({ skipGeminiInitialization: true });
// ACP forwards session messages straight to the model; under progressive
// MCP availability `initialize()` returns before MCP servers settle, so
// we wait here to keep the first session's tool surface consistent with
Expand Down Expand Up @@ -116,10 +115,11 @@ export async function runAcpAgent(
console.debug = console.error;

const stream = ndJsonStream(stdout, stdin);
const connection = new AgentSideConnection(
(conn) => new QwenAgent(config, settings, argv, conn),
stream,
);
let agentInstance: QwenAgent | undefined;
const connection = new AgentSideConnection((conn) => {
agentInstance = new QwenAgent(config, settings, argv, conn);
return agentInstance;
}, stream);

// Handle SIGTERM/SIGINT for graceful shutdown.
// Without this, signal handlers registered elsewhere in the CLI
Expand All @@ -133,9 +133,28 @@ export async function runAcpAgent(
const fireSessionEndOnce = async (reason: SessionEndReason) => {
if (sessionEndFired) return;
sessionEndFired = true;
const hookSystem = config.getHookSystem?.();
const hooksEnabled = !config.getDisableAllHooks?.();
if (hooksEnabled && hookSystem && config.hasHooksForEvent?.('SessionEnd')) {

const configs = new Set<Config>([config]);
const sessions = agentInstance?.getActiveSessions();
if (sessions) {
for (const session of sessions) {
const sessionConfig = session.getConfig?.();
if (sessionConfig) {
configs.add(sessionConfig);
}
}
}

for (const cfg of configs) {
const hookSystem = cfg.getHookSystem?.();
const hooksEnabled = !cfg.getDisableAllHooks?.();
if (
!hooksEnabled ||
!hookSystem ||
!cfg.hasHooksForEvent?.('SessionEnd')
) {
continue;
}
try {
await hookSystem.fireSessionEndEvent(reason);
} catch (err) {
Expand Down Expand Up @@ -215,6 +234,10 @@ class QwenAgent implements Agent {
private sessions: Map<string, Session> = new Map();
private clientCapabilities: ClientCapabilities | undefined;

getActiveSessions(): Session[] {
return [...this.sessions.values()];
}

constructor(
private config: Config,
private settings: LoadedSettings,
Expand Down Expand Up @@ -795,8 +818,9 @@ class QwenAgent implements Agent {
): Promise<Session> {
const sessionId = config.getSessionId();
const geminiClient = config.getGeminiClient();
const needsInitialize = !geminiClient.isInitialized();

if (!geminiClient.isInitialized()) {
if (needsInitialize) {
await geminiClient.initialize();
}

Expand All @@ -808,24 +832,6 @@ class QwenAgent implements Agent {
);
this.sessions.set(sessionId, session);

Comment thread
DennisYu07 marked this conversation as resolved.
// Fire SessionStart hook (aligned with core path)
const hookSystem = config.getHookSystem();
const hooksEnabled = !config.getDisableAllHooks();
if (hooksEnabled && hookSystem && config.hasHooksForEvent('SessionStart')) {
const source = conversation
? SessionStartSource.Resume
: SessionStartSource.Startup;
const model = config.getModel();
const permissionMode = String(config.getApprovalMode()) as PermissionMode;
try {
await hookSystem.fireSessionStartEvent(source, model, permissionMode);
} catch (err) {
debugLogger.warn(
`SessionStart hook failed: ${err instanceof Error ? err.message : String(err)}`,
);
}
}

setTimeout(async () => {
await session.sendAvailableCommandsUpdate();
}, 0);
Expand Down
28 changes: 0 additions & 28 deletions packages/cli/src/ui/AppContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import {
ShellExecutionService,
Storage,
SessionEndReason,
SessionStartSource,
generatePromptSuggestion,
logPromptSuggestion,
PromptSuggestionEvent,
Expand All @@ -56,7 +55,6 @@ import {
ApprovalMode,
ConditionalRulesRegistry,
MCPDiscoveryState,
type PermissionMode,
ToolConfirmationOutcome,
type WaitingToolCall,
ToolNames,
Expand Down Expand Up @@ -487,32 +485,6 @@ export const AppContainer = (props: AppContainerProps) => {
setSessionName(title);
}
}

// Fire SessionStart event after config is initialized
const sessionStartSource = resumedSessionData
? SessionStartSource.Resume
: SessionStartSource.Startup;

const hookSystem = config.getHookSystem();

if (hookSystem) {
hookSystem
.fireSessionStartEvent(
sessionStartSource,
config.getModel() ?? '',
String(config.getApprovalMode()) as PermissionMode,
)
.then(() => {
debugLogger.debug('SessionStart event completed successfully');
})
.catch((err) => {
debugLogger.warn(`SessionStart hook failed: ${err}`);
});
} else {
debugLogger.debug(
'SessionStart: HookSystem not available, skipping event',
);
}
})();

// Register SessionEnd cleanup for process exit
Expand Down
Loading
Loading