move move notification auth_success from useAuth to config to support both interactive and non-interactive

This commit is contained in:
DennisYu07 2026-03-10 20:50:03 -07:00
parent 4b80e4a3b7
commit d5d7187479
4 changed files with 97 additions and 28 deletions

View file

@ -15,8 +15,6 @@ import {
AuthType,
getErrorMessage,
logAuth,
fireNotificationHook,
NotificationType,
} from '@qwen-code/qwen-code-core';
import { useCallback, useEffect, useState } from 'react';
import type { LoadedSettings } from '../../config/settings.js';
@ -170,20 +168,8 @@ export const useAuthCommand = (
const authEvent = new AuthEvent(authType, 'manual', 'success');
logAuth(config, authEvent);
// Fire auth_success notification hook
const messageBus = config.getMessageBus();
const hooksEnabled = config.getEnableHooks();
if (hooksEnabled && messageBus) {
fireNotificationHook(
messageBus,
`Successfully authenticated with ${authType}`,
NotificationType.AuthSuccess,
'Authentication successful',
).catch(() => {
// Silently ignore errors - fireNotificationHook has internal error handling
// and notification hooks should not block the auth flow
});
}
// Note: auth_success notification hook is now fired inside config.refreshAuth()
// to ensure consistent behavior across interactive and non-interactive modes
},
[settings, handleAuthFailure, config, addItem, onAuthChange],
);

View file

@ -36,6 +36,8 @@ import { RipGrepTool } from '../tools/ripGrep.js';
import { logRipgrepFallback } from '../telemetry/loggers.js';
import { RipgrepFallbackEvent } from '../telemetry/types.js';
import { ToolRegistry } from '../tools/tool-registry.js';
import { fireNotificationHook } from '../core/toolHookTriggers.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
function createToolMock(toolName: string) {
const ToolMock = vi.fn();
@ -195,6 +197,10 @@ vi.mock('../ide/ide-client.js', () => ({
import { BaseLlmClient } from '../core/baseLlmClient.js';
vi.mock('../core/baseLlmClient.js');
// Mock fireNotificationHook from toolHookTriggers
vi.mock('../core/toolHookTriggers.js', () => ({
fireNotificationHook: vi.fn().mockResolvedValue({}),
}));
describe('Server Config (config.ts)', () => {
const MODEL = 'qwen3-coder-plus';
@ -317,6 +323,64 @@ describe('Server Config (config.ts)', () => {
expect(GeminiClient).toHaveBeenCalledWith(config);
});
it('should fire auth_success notification hook when hooks are enabled', async () => {
const mockMessageBus = { request: vi.fn() };
const config = new Config({
...baseParams,
enableHooks: true,
});
// Set messageBus using the setter
config.setMessageBus(mockMessageBus as unknown as MessageBus);
const authType = AuthType.USE_GEMINI;
const mockContentConfig = {
apiKey: 'test-key',
model: 'qwen3-coder-plus',
authType,
};
vi.mocked(resolveContentGeneratorConfigWithSources).mockReturnValue({
config: mockContentConfig as ContentGeneratorConfig,
sources: {},
});
await config.refreshAuth(authType);
// Verify that fireNotificationHook was called with correct parameters
expect(fireNotificationHook).toHaveBeenCalledWith(
mockMessageBus,
`Successfully authenticated with ${authType}`,
'auth_success',
'Authentication successful',
);
});
it('should not fire notification hook when hooks are disabled', async () => {
const config = new Config({
...baseParams,
enableHooks: false,
});
const authType = AuthType.USE_GEMINI;
const mockContentConfig = {
apiKey: 'test-key',
model: 'qwen3-coder-plus',
authType,
};
vi.mocked(resolveContentGeneratorConfigWithSources).mockReturnValue({
config: mockContentConfig as ContentGeneratorConfig,
sources: {},
});
// Clear any previous calls
vi.mocked(fireNotificationHook).mockClear();
await config.refreshAuth(authType);
// Verify that fireNotificationHook was not called
expect(fireNotificationHook).not.toHaveBeenCalled();
});
it('should not strip thoughts when switching from Vertex to GenAI', async () => {
const config = new Config(baseParams);

View file

@ -94,9 +94,10 @@ import {
} from '../confirmation-bus/types.js';
import {
PermissionMode,
type NotificationType,
NotificationType,
type PermissionSuggestion,
} from '../hooks/types.js';
import { fireNotificationHook } from '../core/toolHookTriggers.js';
// Utils
import { shouldAttemptBrowserLaunch } from '../utils/browser.js';
@ -982,6 +983,21 @@ export class Config {
// Initialize BaseLlmClient now that the ContentGenerator is available
this.baseLlmClient = new BaseLlmClient(this.contentGenerator, this);
// Fire auth_success notification hook (supports both interactive & non-interactive)
const messageBus = this.getMessageBus();
const hooksEnabled = this.getEnableHooks();
if (hooksEnabled && messageBus) {
fireNotificationHook(
messageBus,
`Successfully authenticated with ${authMethod}`,
NotificationType.AuthSuccess,
'Authentication successful',
).catch(() => {
// Silently ignore errors - fireNotificationHook has internal error handling
// and notification hooks should not block the auth flow
});
}
}
/**

View file

@ -42,6 +42,7 @@ import * as path from 'node:path';
import { MessageBusType } from '../confirmation-bus/types.js';
import type { HookExecutionResponse } from '../confirmation-bus/types.js';
import { type NotificationType } from '../hooks/types.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
vi.mock('fs/promises', () => ({
writeFile: vi.fn(),
@ -2858,7 +2859,7 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => {
// Integration tests for the fire* functions
describe('Fire hook functions integration', () => {
let mockMessageBus: { request: Mock };
let mockMessageBus: { request: ReturnType<typeof vi.fn> };
beforeEach(() => {
mockMessageBus = {
@ -2882,7 +2883,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await firePreToolUseHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'testTool',
{ param: 'value' },
'toolu_test',
@ -2921,7 +2922,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await firePreToolUseHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'testTool',
{ param: 'value' },
'toolu_test',
@ -2952,7 +2953,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockRejectedValue(new Error('Network error'));
const result = await firePreToolUseHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'testTool',
{ param: 'value' },
'toolu_test',
@ -2968,6 +2969,8 @@ describe('Fire hook functions integration', () => {
const { firePostToolUseHook } = await import('./toolHookTriggers.js');
const mockResponse: HookExecutionResponse = {
type: MessageBusType.HOOK_EXECUTION_RESPONSE,
correlationId: 'test-correlation-id',
success: true,
output: {
permission_decision: 'proceed',
@ -2977,7 +2980,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await firePostToolUseHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'testTool',
{ param: 'value' },
{ response: 'result' },
@ -3005,7 +3008,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await firePostToolUseHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'testTool',
{ param: 'value' },
{ response: 'result' },
@ -3053,7 +3056,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await firePostToolUseFailureHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'toolu_test',
'testTool',
{ param: 'value' },
@ -3102,7 +3105,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await fireNotificationHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'Test message',
'info' as NotificationType,
'Test Title',
@ -3155,7 +3158,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await firePermissionRequestHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'testTool',
{ param: 'value' },
'full',
@ -3186,7 +3189,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await firePermissionRequestHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'testTool',
{ param: 'value' },
'full',
@ -3220,7 +3223,7 @@ describe('Fire hook functions integration', () => {
mockMessageBus.request.mockResolvedValue(mockResponse);
const result = await firePermissionRequestHook(
mockMessageBus,
mockMessageBus as unknown as MessageBus,
'testTool',
{ param: 'value' },
'full',