mirror of
https://github.com/QwenLM/qwen-code.git
synced 2025-12-19 09:33:53 +00:00
Merge pull request #1194 from afarber/add-audio-notification-bell
feat: add terminal bell setting to enable/disable audio notifications
This commit is contained in:
@@ -194,6 +194,16 @@ const SETTINGS_SCHEMA = {
|
||||
{ value: 'ru', label: 'Русский (Russian)' },
|
||||
],
|
||||
},
|
||||
terminalBell: {
|
||||
type: 'boolean',
|
||||
label: 'Terminal Bell',
|
||||
category: 'General',
|
||||
requiresRestart: false,
|
||||
default: true,
|
||||
description:
|
||||
'Play terminal bell sound when response completes or needs approval.',
|
||||
showInDialog: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
output: {
|
||||
|
||||
@@ -942,6 +942,7 @@ export const AppContainer = (props: AppContainerProps) => {
|
||||
isFocused,
|
||||
streamingState,
|
||||
elapsedTime,
|
||||
settings,
|
||||
});
|
||||
|
||||
// Dialog close functionality
|
||||
|
||||
@@ -15,6 +15,23 @@ import {
|
||||
LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS,
|
||||
useAttentionNotifications,
|
||||
} from './useAttentionNotifications.js';
|
||||
import type { LoadedSettings } from '../../config/settings.js';
|
||||
|
||||
const mockSettings: LoadedSettings = {
|
||||
merged: {
|
||||
general: {
|
||||
terminalBell: true,
|
||||
},
|
||||
},
|
||||
} as LoadedSettings;
|
||||
|
||||
const mockSettingsDisabled: LoadedSettings = {
|
||||
merged: {
|
||||
general: {
|
||||
terminalBell: false,
|
||||
},
|
||||
},
|
||||
} as LoadedSettings;
|
||||
|
||||
vi.mock('../../utils/attentionNotification.js', () => ({
|
||||
notifyTerminalAttention: vi.fn(),
|
||||
@@ -40,6 +57,7 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: true,
|
||||
streamingState: StreamingState.Idle,
|
||||
elapsedTime: 0,
|
||||
settings: mockSettings,
|
||||
...props,
|
||||
},
|
||||
},
|
||||
@@ -53,11 +71,13 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: false,
|
||||
streamingState: StreamingState.WaitingForConfirmation,
|
||||
elapsedTime: 0,
|
||||
settings: mockSettings,
|
||||
},
|
||||
});
|
||||
|
||||
expect(mockedNotify).toHaveBeenCalledWith(
|
||||
AttentionNotificationReason.ToolApproval,
|
||||
{ enabled: true },
|
||||
);
|
||||
});
|
||||
|
||||
@@ -72,6 +92,7 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: false,
|
||||
streamingState: StreamingState.WaitingForConfirmation,
|
||||
elapsedTime: 0,
|
||||
settings: mockSettings,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -86,6 +107,7 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: false,
|
||||
streamingState: StreamingState.Responding,
|
||||
elapsedTime: LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS + 5,
|
||||
settings: mockSettings,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -94,11 +116,13 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: false,
|
||||
streamingState: StreamingState.Idle,
|
||||
elapsedTime: 0,
|
||||
settings: mockSettings,
|
||||
},
|
||||
});
|
||||
|
||||
expect(mockedNotify).toHaveBeenCalledWith(
|
||||
AttentionNotificationReason.LongTaskComplete,
|
||||
{ enabled: true },
|
||||
);
|
||||
});
|
||||
|
||||
@@ -110,6 +134,7 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: true,
|
||||
streamingState: StreamingState.Responding,
|
||||
elapsedTime: LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS + 2,
|
||||
settings: mockSettings,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -118,6 +143,7 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: true,
|
||||
streamingState: StreamingState.Idle,
|
||||
elapsedTime: 0,
|
||||
settings: mockSettings,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -135,6 +161,7 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: false,
|
||||
streamingState: StreamingState.Responding,
|
||||
elapsedTime: 5,
|
||||
settings: mockSettings,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -143,9 +170,30 @@ describe('useAttentionNotifications', () => {
|
||||
isFocused: false,
|
||||
streamingState: StreamingState.Idle,
|
||||
elapsedTime: 0,
|
||||
settings: mockSettings,
|
||||
},
|
||||
});
|
||||
|
||||
expect(mockedNotify).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not notify when terminalBell setting is disabled', () => {
|
||||
const { rerender } = render({
|
||||
settings: mockSettingsDisabled,
|
||||
});
|
||||
|
||||
rerender({
|
||||
hookProps: {
|
||||
isFocused: false,
|
||||
streamingState: StreamingState.WaitingForConfirmation,
|
||||
elapsedTime: 0,
|
||||
settings: mockSettingsDisabled,
|
||||
},
|
||||
});
|
||||
|
||||
expect(mockedNotify).toHaveBeenCalledWith(
|
||||
AttentionNotificationReason.ToolApproval,
|
||||
{ enabled: false },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
notifyTerminalAttention,
|
||||
AttentionNotificationReason,
|
||||
} from '../../utils/attentionNotification.js';
|
||||
import type { LoadedSettings } from '../../config/settings.js';
|
||||
|
||||
export const LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS = 20;
|
||||
|
||||
@@ -17,13 +18,16 @@ interface UseAttentionNotificationsOptions {
|
||||
isFocused: boolean;
|
||||
streamingState: StreamingState;
|
||||
elapsedTime: number;
|
||||
settings: LoadedSettings;
|
||||
}
|
||||
|
||||
export const useAttentionNotifications = ({
|
||||
isFocused,
|
||||
streamingState,
|
||||
elapsedTime,
|
||||
settings,
|
||||
}: UseAttentionNotificationsOptions) => {
|
||||
const terminalBellEnabled = settings?.merged?.general?.terminalBell ?? true;
|
||||
const awaitingNotificationSentRef = useRef(false);
|
||||
const respondingElapsedRef = useRef(0);
|
||||
|
||||
@@ -33,14 +37,16 @@ export const useAttentionNotifications = ({
|
||||
!isFocused &&
|
||||
!awaitingNotificationSentRef.current
|
||||
) {
|
||||
notifyTerminalAttention(AttentionNotificationReason.ToolApproval);
|
||||
notifyTerminalAttention(AttentionNotificationReason.ToolApproval, {
|
||||
enabled: terminalBellEnabled,
|
||||
});
|
||||
awaitingNotificationSentRef.current = true;
|
||||
}
|
||||
|
||||
if (streamingState !== StreamingState.WaitingForConfirmation || isFocused) {
|
||||
awaitingNotificationSentRef.current = false;
|
||||
}
|
||||
}, [isFocused, streamingState]);
|
||||
}, [isFocused, streamingState, terminalBellEnabled]);
|
||||
|
||||
useEffect(() => {
|
||||
if (streamingState === StreamingState.Responding) {
|
||||
@@ -53,11 +59,13 @@ export const useAttentionNotifications = ({
|
||||
respondingElapsedRef.current >=
|
||||
LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS;
|
||||
if (wasLongTask && !isFocused) {
|
||||
notifyTerminalAttention(AttentionNotificationReason.LongTaskComplete);
|
||||
notifyTerminalAttention(AttentionNotificationReason.LongTaskComplete, {
|
||||
enabled: terminalBellEnabled,
|
||||
});
|
||||
}
|
||||
// Reset tracking for next task
|
||||
respondingElapsedRef.current = 0;
|
||||
return;
|
||||
}
|
||||
}, [streamingState, elapsedTime, isFocused]);
|
||||
}, [streamingState, elapsedTime, isFocused, terminalBellEnabled]);
|
||||
};
|
||||
|
||||
@@ -13,6 +13,7 @@ export enum AttentionNotificationReason {
|
||||
|
||||
export interface TerminalNotificationOptions {
|
||||
stream?: Pick<NodeJS.WriteStream, 'write' | 'isTTY'>;
|
||||
enabled?: boolean;
|
||||
}
|
||||
|
||||
const TERMINAL_BELL = '\u0007';
|
||||
@@ -28,6 +29,11 @@ export function notifyTerminalAttention(
|
||||
_reason: AttentionNotificationReason,
|
||||
options: TerminalNotificationOptions = {},
|
||||
): boolean {
|
||||
// Check if terminal bell is enabled (default true for backwards compatibility)
|
||||
if (options.enabled === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const stream = options.stream ?? process.stdout;
|
||||
if (!stream?.write || stream.isTTY === false) {
|
||||
return false;
|
||||
|
||||
Reference in New Issue
Block a user