From 5bc309b3dc237e6a279037531a9889bd0ff84a53 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Tue, 18 Nov 2025 13:43:17 +0800 Subject: [PATCH 1/5] feat: add os platform and version in log report (#1053) --- .../src/telemetry/qwen-logger/event-types.ts | 17 ++++++++++++++ .../telemetry/qwen-logger/qwen-logger.test.ts | 23 ++++++++++++++++++- .../src/telemetry/qwen-logger/qwen-logger.ts | 11 +++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/packages/core/src/telemetry/qwen-logger/event-types.ts b/packages/core/src/telemetry/qwen-logger/event-types.ts index f81fb712..ed84ba78 100644 --- a/packages/core/src/telemetry/qwen-logger/event-types.ts +++ b/packages/core/src/telemetry/qwen-logger/event-types.ts @@ -19,6 +19,21 @@ export interface RumView { name: string; } +export interface RumOS { + type?: string; + version?: string; + container?: string; + container_version?: string; +} + +export interface RumDevice { + id?: string; + name?: string; + type?: string; + brand?: string; + model?: string; +} + export interface RumEvent { timestamp?: number; event_type?: 'view' | 'action' | 'exception' | 'resource'; @@ -78,6 +93,8 @@ export interface RumPayload { user: RumUser; session: RumSession; view: RumView; + os?: RumOS; + device?: RumDevice; events: RumEvent[]; properties?: Record; _v: string; diff --git a/packages/core/src/telemetry/qwen-logger/qwen-logger.test.ts b/packages/core/src/telemetry/qwen-logger/qwen-logger.test.ts index 8efb2e4e..9dbaa4f9 100644 --- a/packages/core/src/telemetry/qwen-logger/qwen-logger.test.ts +++ b/packages/core/src/telemetry/qwen-logger/qwen-logger.test.ts @@ -13,8 +13,10 @@ import { afterEach, afterAll, } from 'vitest'; +import * as os from 'node:os'; import { QwenLogger, TEST_ONLY } from './qwen-logger.js'; import type { Config } from '../../config/config.js'; +import { AuthType } from '../../core/contentGenerator.js'; import { StartSessionEvent, EndSessionEvent, @@ -22,7 +24,7 @@ import { KittySequenceOverflowEvent, IdeConnectionType, } from '../types.js'; -import type { RumEvent } from './event-types.js'; +import type { RumEvent, RumPayload } from './event-types.js'; // Mock dependencies vi.mock('../../utils/user_id.js', () => ({ @@ -46,6 +48,7 @@ const makeFakeConfig = (overrides: Partial = {}): Config => { getCliVersion: () => '1.0.0', getProxy: () => undefined, getContentGeneratorConfig: () => ({ authType: 'test-auth' }), + getAuthType: () => AuthType.QWEN_OAUTH, getMcpServers: () => ({}), getModel: () => 'test-model', getEmbeddingModel: () => 'test-embedding', @@ -102,6 +105,24 @@ describe('QwenLogger', () => { }); }); + describe('createRumPayload', () => { + it('includes os metadata in payload', async () => { + const logger = QwenLogger.getInstance(mockConfig)!; + const payload = await ( + logger as unknown as { + createRumPayload(): Promise; + } + ).createRumPayload(); + + expect(payload.os).toEqual( + expect.objectContaining({ + type: os.platform(), + version: os.release(), + }), + ); + }); + }); + describe('event queue management', () => { it('should handle event overflow gracefully', () => { const debugConfig = makeFakeConfig({ getDebugMode: () => true }); diff --git a/packages/core/src/telemetry/qwen-logger/qwen-logger.ts b/packages/core/src/telemetry/qwen-logger/qwen-logger.ts index 96f796e4..5f9a2a51 100644 --- a/packages/core/src/telemetry/qwen-logger/qwen-logger.ts +++ b/packages/core/src/telemetry/qwen-logger/qwen-logger.ts @@ -6,6 +6,7 @@ import { Buffer } from 'buffer'; import * as https from 'https'; +import * as os from 'node:os'; import { HttpsProxyAgent } from 'https-proxy-agent'; import type { @@ -45,6 +46,7 @@ import type { RumResourceEvent, RumExceptionEvent, RumPayload, + RumOS, } from './event-types.js'; import type { Config } from '../../config/config.js'; import { safeJsonStringify } from '../../utils/safeJsonStringify.js'; @@ -214,9 +216,17 @@ export class QwenLogger { return this.createRumEvent('exception', type, name, properties); } + private getOsMetadata(): RumOS { + return { + type: os.platform(), + version: os.release(), + }; + } + async createRumPayload(): Promise { const authType = this.config?.getAuthType(); const version = this.config?.getCliVersion() || 'unknown'; + const osMetadata = this.getOsMetadata(); return { app: { @@ -235,6 +245,7 @@ export class QwenLogger { id: this.sessionId, name: 'qwen-code-cli', }, + os: osMetadata, events: this.events.toArray() as RumEvent[], properties: { From 6bb829f87641deb6ef7717727bae207803938749 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Tue, 18 Nov 2025 13:43:43 +0800 Subject: [PATCH 2/5] feat: Add Terminal Attention Notifications for User Alerts (#1052) --- packages/cli/src/ui/AppContainer.tsx | 7 + .../hooks/useAttentionNotifications.test.ts | 151 ++++++++++++++++++ .../src/ui/hooks/useAttentionNotifications.ts | 63 ++++++++ .../src/utils/attentionNotification.test.ts | 72 +++++++++ .../cli/src/utils/attentionNotification.ts | 43 +++++ 5 files changed, 336 insertions(+) create mode 100644 packages/cli/src/ui/hooks/useAttentionNotifications.test.ts create mode 100644 packages/cli/src/ui/hooks/useAttentionNotifications.ts create mode 100644 packages/cli/src/utils/attentionNotification.test.ts create mode 100644 packages/cli/src/utils/attentionNotification.ts diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index fbfc732b..c3443b43 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -97,6 +97,7 @@ import { type VisionSwitchOutcome } from './components/ModelSwitchDialog.js'; import { processVisionSwitchOutcome } from './hooks/useVisionAutoSwitch.js'; import { useSubagentCreateDialog } from './hooks/useSubagentCreateDialog.js'; import { useAgentsManagerDialog } from './hooks/useAgentsManagerDialog.js'; +import { useAttentionNotifications } from './hooks/useAttentionNotifications.js'; const CTRL_EXIT_PROMPT_DURATION_MS = 1000; @@ -944,6 +945,12 @@ export const AppContainer = (props: AppContainerProps) => { settings.merged.ui?.customWittyPhrases, ); + useAttentionNotifications({ + isFocused, + streamingState, + elapsedTime, + }); + // Dialog close functionality const { closeAnyOpenDialog } = useDialogClose({ isThemeDialogOpen, diff --git a/packages/cli/src/ui/hooks/useAttentionNotifications.test.ts b/packages/cli/src/ui/hooks/useAttentionNotifications.test.ts new file mode 100644 index 00000000..1475aa52 --- /dev/null +++ b/packages/cli/src/ui/hooks/useAttentionNotifications.test.ts @@ -0,0 +1,151 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { renderHook } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { StreamingState } from '../types.js'; +import { + AttentionNotificationReason, + notifyTerminalAttention, +} from '../../utils/attentionNotification.js'; +import { + LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS, + useAttentionNotifications, +} from './useAttentionNotifications.js'; + +vi.mock('../../utils/attentionNotification.js', () => ({ + notifyTerminalAttention: vi.fn(), + AttentionNotificationReason: { + ToolApproval: 'tool_approval', + LongTaskComplete: 'long_task_complete', + }, +})); + +const mockedNotify = vi.mocked(notifyTerminalAttention); + +describe('useAttentionNotifications', () => { + beforeEach(() => { + mockedNotify.mockReset(); + }); + + const render = ( + props?: Partial[0]>, + ) => + renderHook(({ hookProps }) => useAttentionNotifications(hookProps), { + initialProps: { + hookProps: { + isFocused: true, + streamingState: StreamingState.Idle, + elapsedTime: 0, + ...props, + }, + }, + }); + + it('notifies when tool approval is required while unfocused', () => { + const { rerender } = render(); + + rerender({ + hookProps: { + isFocused: false, + streamingState: StreamingState.WaitingForConfirmation, + elapsedTime: 0, + }, + }); + + expect(mockedNotify).toHaveBeenCalledWith( + AttentionNotificationReason.ToolApproval, + ); + }); + + it('notifies when focus is lost after entering approval wait state', () => { + const { rerender } = render({ + isFocused: true, + streamingState: StreamingState.WaitingForConfirmation, + }); + + rerender({ + hookProps: { + isFocused: false, + streamingState: StreamingState.WaitingForConfirmation, + elapsedTime: 0, + }, + }); + + expect(mockedNotify).toHaveBeenCalledTimes(1); + }); + + it('sends a notification when a long task finishes while unfocused', () => { + const { rerender } = render(); + + rerender({ + hookProps: { + isFocused: false, + streamingState: StreamingState.Responding, + elapsedTime: LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS + 5, + }, + }); + + rerender({ + hookProps: { + isFocused: false, + streamingState: StreamingState.Idle, + elapsedTime: 0, + }, + }); + + expect(mockedNotify).toHaveBeenCalledWith( + AttentionNotificationReason.LongTaskComplete, + ); + }); + + it('does not notify about long tasks when the CLI is focused', () => { + const { rerender } = render(); + + rerender({ + hookProps: { + isFocused: true, + streamingState: StreamingState.Responding, + elapsedTime: LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS + 2, + }, + }); + + rerender({ + hookProps: { + isFocused: true, + streamingState: StreamingState.Idle, + elapsedTime: 0, + }, + }); + + expect(mockedNotify).not.toHaveBeenCalledWith( + AttentionNotificationReason.LongTaskComplete, + expect.anything(), + ); + }); + + it('does not treat short responses as long tasks', () => { + const { rerender } = render(); + + rerender({ + hookProps: { + isFocused: false, + streamingState: StreamingState.Responding, + elapsedTime: 5, + }, + }); + + rerender({ + hookProps: { + isFocused: false, + streamingState: StreamingState.Idle, + elapsedTime: 0, + }, + }); + + expect(mockedNotify).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/ui/hooks/useAttentionNotifications.ts b/packages/cli/src/ui/hooks/useAttentionNotifications.ts new file mode 100644 index 00000000..e632c827 --- /dev/null +++ b/packages/cli/src/ui/hooks/useAttentionNotifications.ts @@ -0,0 +1,63 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useEffect, useRef } from 'react'; +import { StreamingState } from '../types.js'; +import { + notifyTerminalAttention, + AttentionNotificationReason, +} from '../../utils/attentionNotification.js'; + +export const LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS = 20; + +interface UseAttentionNotificationsOptions { + isFocused: boolean; + streamingState: StreamingState; + elapsedTime: number; +} + +export const useAttentionNotifications = ({ + isFocused, + streamingState, + elapsedTime, +}: UseAttentionNotificationsOptions) => { + const awaitingNotificationSentRef = useRef(false); + const respondingElapsedRef = useRef(0); + + useEffect(() => { + if ( + streamingState === StreamingState.WaitingForConfirmation && + !isFocused && + !awaitingNotificationSentRef.current + ) { + notifyTerminalAttention(AttentionNotificationReason.ToolApproval); + awaitingNotificationSentRef.current = true; + } + + if (streamingState !== StreamingState.WaitingForConfirmation || isFocused) { + awaitingNotificationSentRef.current = false; + } + }, [isFocused, streamingState]); + + useEffect(() => { + if (streamingState === StreamingState.Responding) { + respondingElapsedRef.current = elapsedTime; + return; + } + + if (streamingState === StreamingState.Idle) { + const wasLongTask = + respondingElapsedRef.current >= + LONG_TASK_NOTIFICATION_THRESHOLD_SECONDS; + if (wasLongTask && !isFocused) { + notifyTerminalAttention(AttentionNotificationReason.LongTaskComplete); + } + // Reset tracking for next task + respondingElapsedRef.current = 0; + return; + } + }, [streamingState, elapsedTime, isFocused]); +}; diff --git a/packages/cli/src/utils/attentionNotification.test.ts b/packages/cli/src/utils/attentionNotification.test.ts new file mode 100644 index 00000000..9ebb785c --- /dev/null +++ b/packages/cli/src/utils/attentionNotification.test.ts @@ -0,0 +1,72 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + notifyTerminalAttention, + AttentionNotificationReason, +} from './attentionNotification.js'; + +describe('notifyTerminalAttention', () => { + let stream: { write: ReturnType; isTTY: boolean }; + + beforeEach(() => { + stream = { write: vi.fn().mockReturnValue(true), isTTY: true }; + }); + + it('emits terminal bell character', () => { + const result = notifyTerminalAttention( + AttentionNotificationReason.ToolApproval, + { + stream, + }, + ); + + expect(result).toBe(true); + expect(stream.write).toHaveBeenCalledWith('\u0007'); + }); + + it('returns false when not running inside a tty', () => { + stream.isTTY = false; + + const result = notifyTerminalAttention( + AttentionNotificationReason.ToolApproval, + { stream }, + ); + + expect(result).toBe(false); + expect(stream.write).not.toHaveBeenCalled(); + }); + + it('returns false when stream write fails', () => { + stream.write = vi.fn().mockImplementation(() => { + throw new Error('Write failed'); + }); + + const result = notifyTerminalAttention( + AttentionNotificationReason.ToolApproval, + { stream }, + ); + + expect(result).toBe(false); + }); + + it('works with different notification reasons', () => { + const reasons = [ + AttentionNotificationReason.ToolApproval, + AttentionNotificationReason.LongTaskComplete, + ]; + + reasons.forEach((reason) => { + stream.write.mockClear(); + + const result = notifyTerminalAttention(reason, { stream }); + + expect(result).toBe(true); + expect(stream.write).toHaveBeenCalledWith('\u0007'); + }); + }); +}); diff --git a/packages/cli/src/utils/attentionNotification.ts b/packages/cli/src/utils/attentionNotification.ts new file mode 100644 index 00000000..26dc2a25 --- /dev/null +++ b/packages/cli/src/utils/attentionNotification.ts @@ -0,0 +1,43 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import process from 'node:process'; + +export enum AttentionNotificationReason { + ToolApproval = 'tool_approval', + LongTaskComplete = 'long_task_complete', +} + +export interface TerminalNotificationOptions { + stream?: Pick; +} + +const TERMINAL_BELL = '\u0007'; + +/** + * Grabs the user's attention by emitting the terminal bell character. + * This causes the terminal to flash or play a sound, alerting the user + * to check the CLI for important events. + * + * @returns true when the bell was successfully written to the terminal. + */ +export function notifyTerminalAttention( + _reason: AttentionNotificationReason, + options: TerminalNotificationOptions = {}, +): boolean { + const stream = options.stream ?? process.stdout; + if (!stream?.write || stream.isTTY === false) { + return false; + } + + try { + stream.write(TERMINAL_BELL); + return true; + } catch (error) { + console.warn('Failed to send terminal bell:', error); + return false; + } +} From efca0bc7951eee3e051d5dd3688b1e357bd269cb Mon Sep 17 00:00:00 2001 From: Mingholy Date: Tue, 18 Nov 2025 13:46:42 +0800 Subject: [PATCH 3/5] fix: basic slash command support (#1020) --- packages/cli/src/nonInteractiveCli.test.ts | 5 + packages/cli/src/nonInteractiveCliCommands.ts | 104 ++++++++++++++- packages/cli/src/zed-integration/schema.ts | 24 ++++ .../cli/src/zed-integration/zedIntegration.ts | 123 +++++++++++++++++- 4 files changed, 247 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index 1aad835e..066b1848 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -23,6 +23,7 @@ import type { Part } from '@google/genai'; import { runNonInteractive } from './nonInteractiveCli.js'; import { vi } from 'vitest'; import type { LoadedSettings } from './config/settings.js'; +import { CommandKind } from './ui/commands/types.js'; // Mock core modules vi.mock('./ui/hooks/atCommandProcessor.js'); @@ -727,6 +728,7 @@ describe('runNonInteractive', () => { const mockCommand = { name: 'testcommand', description: 'a test command', + kind: CommandKind.FILE, action: vi.fn().mockResolvedValue({ type: 'submit_prompt', content: [{ text: 'Prompt from command' }], @@ -766,6 +768,7 @@ describe('runNonInteractive', () => { const mockCommand = { name: 'confirm', description: 'a command that needs confirmation', + kind: CommandKind.FILE, action: vi.fn().mockResolvedValue({ type: 'confirm_shell_commands', commands: ['rm -rf /'], @@ -821,6 +824,7 @@ describe('runNonInteractive', () => { const mockCommand = { name: 'noaction', description: 'unhandled type', + kind: CommandKind.FILE, action: vi.fn().mockResolvedValue({ type: 'unhandled', }), @@ -847,6 +851,7 @@ describe('runNonInteractive', () => { const mockCommand = { name: 'testargs', description: 'a test command', + kind: CommandKind.FILE, action: mockAction, }; mockGetCommands.mockReturnValue([mockCommand]); diff --git a/packages/cli/src/nonInteractiveCliCommands.ts b/packages/cli/src/nonInteractiveCliCommands.ts index 166a1706..77b9d099 100644 --- a/packages/cli/src/nonInteractiveCliCommands.ts +++ b/packages/cli/src/nonInteractiveCliCommands.ts @@ -13,15 +13,56 @@ import { type Config, } from '@qwen-code/qwen-code-core'; import { CommandService } from './services/CommandService.js'; +import { BuiltinCommandLoader } from './services/BuiltinCommandLoader.js'; import { FileCommandLoader } from './services/FileCommandLoader.js'; -import type { CommandContext } from './ui/commands/types.js'; +import { + CommandKind, + type CommandContext, + type SlashCommand, +} from './ui/commands/types.js'; import { createNonInteractiveUI } from './ui/noninteractive/nonInteractiveUi.js'; import type { LoadedSettings } from './config/settings.js'; import type { SessionStatsState } from './ui/contexts/SessionContext.js'; +/** + * Filters commands based on the allowed built-in command names. + * + * - Always includes FILE commands + * - Only includes BUILT_IN commands if their name is in the allowed set + * - Excludes other command types (e.g., MCP_PROMPT) in non-interactive mode + * + * @param commands All loaded commands + * @param allowedBuiltinCommandNames Set of allowed built-in command names (empty = none allowed) + * @returns Filtered commands + */ +function filterCommandsForNonInteractive( + commands: readonly SlashCommand[], + allowedBuiltinCommandNames: Set, +): SlashCommand[] { + return commands.filter((cmd) => { + if (cmd.kind === CommandKind.FILE) { + return true; + } + + // Built-in commands: only include if in the allowed list + if (cmd.kind === CommandKind.BUILT_IN) { + return allowedBuiltinCommandNames.has(cmd.name); + } + + // Exclude other types (e.g., MCP_PROMPT) in non-interactive mode + return false; + }); +} + /** * Processes a slash command in a non-interactive environment. * + * @param rawQuery The raw query string (should start with '/') + * @param abortController Controller to cancel the operation + * @param config The configuration object + * @param settings The loaded settings + * @param allowedBuiltinCommandNames Optional array of built-in command names that are + * allowed. If not provided or empty, only file commands are available. * @returns A Promise that resolves to `PartListUnion` if a valid command is * found and results in a prompt, or `undefined` otherwise. * @throws {FatalInputError} if the command result is not supported in @@ -32,21 +73,35 @@ export const handleSlashCommand = async ( abortController: AbortController, config: Config, settings: LoadedSettings, + allowedBuiltinCommandNames?: string[], ): Promise => { const trimmed = rawQuery.trim(); if (!trimmed.startsWith('/')) { return; } - // Only custom commands are supported for now. - const loaders = [new FileCommandLoader(config)]; + const allowedBuiltinSet = new Set(allowedBuiltinCommandNames ?? []); + + // Only load BuiltinCommandLoader if there are allowed built-in commands + const loaders = + allowedBuiltinSet.size > 0 + ? [new BuiltinCommandLoader(config), new FileCommandLoader(config)] + : [new FileCommandLoader(config)]; + const commandService = await CommandService.create( loaders, abortController.signal, ); const commands = commandService.getCommands(); + const filteredCommands = filterCommandsForNonInteractive( + commands, + allowedBuiltinSet, + ); - const { commandToExecute, args } = parseSlashCommand(rawQuery, commands); + const { commandToExecute, args } = parseSlashCommand( + rawQuery, + filteredCommands, + ); if (commandToExecute) { if (commandToExecute.action) { @@ -107,3 +162,44 @@ export const handleSlashCommand = async ( return; }; + +/** + * Retrieves all available slash commands for the current configuration. + * + * @param config The configuration object + * @param settings The loaded settings + * @param abortSignal Signal to cancel the loading process + * @param allowedBuiltinCommandNames Optional array of built-in command names that are + * allowed. If not provided or empty, only file commands are available. + * @returns A Promise that resolves to an array of SlashCommand objects + */ +export const getAvailableCommands = async ( + config: Config, + settings: LoadedSettings, + abortSignal: AbortSignal, + allowedBuiltinCommandNames?: string[], +): Promise => { + try { + const allowedBuiltinSet = new Set(allowedBuiltinCommandNames ?? []); + + // Only load BuiltinCommandLoader if there are allowed built-in commands + const loaders = + allowedBuiltinSet.size > 0 + ? [new BuiltinCommandLoader(config), new FileCommandLoader(config)] + : [new FileCommandLoader(config)]; + + const commandService = await CommandService.create(loaders, abortSignal); + const commands = commandService.getCommands(); + const filteredCommands = filterCommandsForNonInteractive( + commands, + allowedBuiltinSet, + ); + + // Filter out hidden commands + return filteredCommands.filter((cmd) => !cmd.hidden); + } catch (error) { + // Handle errors gracefully - log and return empty array + console.error('Error loading available commands:', error); + return []; + } +}; diff --git a/packages/cli/src/zed-integration/schema.ts b/packages/cli/src/zed-integration/schema.ts index b35cc47d..e5f72b50 100644 --- a/packages/cli/src/zed-integration/schema.ts +++ b/packages/cli/src/zed-integration/schema.ts @@ -128,6 +128,14 @@ export type AgentRequest = z.infer; export type AgentNotification = z.infer; +export type AvailableCommandInput = z.infer; + +export type AvailableCommand = z.infer; + +export type AvailableCommandsUpdate = z.infer< + typeof availableCommandsUpdateSchema +>; + export const writeTextFileRequestSchema = z.object({ content: z.string(), path: z.string(), @@ -386,6 +394,21 @@ export const promptRequestSchema = z.object({ sessionId: z.string(), }); +export const availableCommandInputSchema = z.object({ + hint: z.string(), +}); + +export const availableCommandSchema = z.object({ + description: z.string(), + input: availableCommandInputSchema.nullable().optional(), + name: z.string(), +}); + +export const availableCommandsUpdateSchema = z.object({ + availableCommands: z.array(availableCommandSchema), + sessionUpdate: z.literal('available_commands_update'), +}); + export const sessionUpdateSchema = z.union([ z.object({ content: contentBlockSchema, @@ -423,6 +446,7 @@ export const sessionUpdateSchema = z.union([ entries: z.array(planEntrySchema), sessionUpdate: z.literal('plan'), }), + availableCommandsUpdateSchema, ]); export const agentResponseSchema = z.union([ diff --git a/packages/cli/src/zed-integration/zedIntegration.ts b/packages/cli/src/zed-integration/zedIntegration.ts index 4a01ed7e..d83395f2 100644 --- a/packages/cli/src/zed-integration/zedIntegration.ts +++ b/packages/cli/src/zed-integration/zedIntegration.ts @@ -31,6 +31,7 @@ import { MCPServerConfig, ToolConfirmationOutcome, logToolCall, + logUserPrompt, getErrorStatus, isWithinRoot, isNodeError, @@ -38,6 +39,7 @@ import { TaskTool, Kind, TodoWriteTool, + UserPromptEvent, } from '@qwen-code/qwen-code-core'; import * as acp from './acp.js'; import { AcpFileSystemService } from './fileSystemService.js'; @@ -53,6 +55,26 @@ import { ExtensionStorage, type Extension } from '../config/extension.js'; import type { CliArgs } from '../config/config.js'; import { loadCliConfig } from '../config/config.js'; import { ExtensionEnablementManager } from '../config/extensions/extensionEnablement.js'; +import { + handleSlashCommand, + getAvailableCommands, +} from '../nonInteractiveCliCommands.js'; +import type { AvailableCommand, AvailableCommandsUpdate } from './schema.js'; +import { isSlashCommand } from '../ui/utils/commandUtils.js'; + +/** + * Built-in commands that are allowed in ACP integration mode. + * Only these commands will be available when using handleSlashCommand + * or getAvailableCommands in ACP integration. + * + * Currently, only "init" is supported because `handleSlashCommand` in + * nonInteractiveCliCommands.ts only supports handling results where + * result.type is "submit_prompt". Other result types are either coupled + * to the UI or cannot send notifications to the client via ACP. + * + * If you have a good idea to add support for more commands, PRs are welcome! + */ +const ALLOWED_BUILTIN_COMMANDS_FOR_ACP = ['init']; /** * Resolves the model to use based on the current configuration. @@ -151,7 +173,7 @@ class GeminiAgent { cwd, mcpServers, }: acp.NewSessionRequest): Promise { - const sessionId = randomUUID(); + const sessionId = this.config.getSessionId() || randomUUID(); const config = await this.newSessionConfig(sessionId, cwd, mcpServers); let isAuthenticated = false; @@ -182,9 +204,20 @@ class GeminiAgent { const geminiClient = config.getGeminiClient(); const chat = await geminiClient.startChat(); - const session = new Session(sessionId, chat, config, this.client); + const session = new Session( + sessionId, + chat, + config, + this.client, + this.settings, + ); this.sessions.set(sessionId, session); + // Send available commands update as the first session update + setTimeout(async () => { + await session.sendAvailableCommandsUpdate(); + }, 0); + return { sessionId, }; @@ -242,12 +275,14 @@ class GeminiAgent { class Session { private pendingPrompt: AbortController | null = null; + private turn: number = 0; constructor( private readonly id: string, private readonly chat: GeminiChat, private readonly config: Config, private readonly client: acp.Client, + private readonly settings: LoadedSettings, ) {} async cancelPendingPrompt(): Promise { @@ -264,10 +299,57 @@ class Session { const pendingSend = new AbortController(); this.pendingPrompt = pendingSend; - const promptId = Math.random().toString(16).slice(2); - const chat = this.chat; + // Increment turn counter for each user prompt + this.turn += 1; - const parts = await this.#resolvePrompt(params.prompt, pendingSend.signal); + const chat = this.chat; + const promptId = this.config.getSessionId() + '########' + this.turn; + + // Extract text from all text blocks to construct the full prompt text for logging + const promptText = params.prompt + .filter((block) => block.type === 'text') + .map((block) => (block.type === 'text' ? block.text : '')) + .join(' '); + + // Log user prompt + logUserPrompt( + this.config, + new UserPromptEvent( + promptText.length, + promptId, + this.config.getContentGeneratorConfig()?.authType, + promptText, + ), + ); + + // Check if the input contains a slash command + // Extract text from the first text block if present + const firstTextBlock = params.prompt.find((block) => block.type === 'text'); + const inputText = firstTextBlock?.text || ''; + + let parts: Part[]; + + if (isSlashCommand(inputText)) { + // Handle slash command - allow specific built-in commands for ACP integration + const slashCommandResult = await handleSlashCommand( + inputText, + pendingSend, + this.config, + this.settings, + ALLOWED_BUILTIN_COMMANDS_FOR_ACP, + ); + + if (slashCommandResult) { + // Use the result from the slash command + parts = slashCommandResult as Part[]; + } else { + // Slash command didn't return a prompt, continue with normal processing + parts = await this.#resolvePrompt(params.prompt, pendingSend.signal); + } + } else { + // Normal processing for non-slash commands + parts = await this.#resolvePrompt(params.prompt, pendingSend.signal); + } let nextMessage: Content | null = { role: 'user', parts }; @@ -361,6 +443,37 @@ class Session { await this.client.sessionUpdate(params); } + async sendAvailableCommandsUpdate(): Promise { + const abortController = new AbortController(); + try { + const slashCommands = await getAvailableCommands( + this.config, + this.settings, + abortController.signal, + ALLOWED_BUILTIN_COMMANDS_FOR_ACP, + ); + + // Convert SlashCommand[] to AvailableCommand[] format for ACP protocol + const availableCommands: AvailableCommand[] = slashCommands.map( + (cmd) => ({ + name: cmd.name, + description: cmd.description, + input: null, + }), + ); + + const update: AvailableCommandsUpdate = { + sessionUpdate: 'available_commands_update', + availableCommands, + }; + + await this.sendUpdate(update); + } catch (error) { + // Log error but don't fail session creation + console.error('Error sending available commands update:', error); + } + } + private async runTool( abortSignal: AbortSignal, promptId: string, From f0bbeac04a10bd5477c0d8944bbea921837c722f Mon Sep 17 00:00:00 2001 From: DS-Controller2 Date: Tue, 18 Nov 2025 00:47:20 -0500 Subject: [PATCH 4/5] fix(core): add modelscope provider to handle stream_options (#848) * fix(core): add modelscope provider to handle stream_options --------- Co-authored-by: Qwen Code Co-authored-by: mingholy.lmh --- .../src/core/openaiContentGenerator/index.ts | 9 ++ .../openaiContentGenerator/provider/index.ts | 1 + .../provider/modelscope.test.ts | 96 +++++++++++++++++++ .../provider/modelscope.ts | 32 +++++++ 4 files changed, 138 insertions(+) create mode 100644 packages/core/src/core/openaiContentGenerator/provider/modelscope.test.ts create mode 100644 packages/core/src/core/openaiContentGenerator/provider/modelscope.ts diff --git a/packages/core/src/core/openaiContentGenerator/index.ts b/packages/core/src/core/openaiContentGenerator/index.ts index 192bf096..8559258c 100644 --- a/packages/core/src/core/openaiContentGenerator/index.ts +++ b/packages/core/src/core/openaiContentGenerator/index.ts @@ -13,6 +13,7 @@ import { OpenAIContentGenerator } from './openaiContentGenerator.js'; import { DashScopeOpenAICompatibleProvider, DeepSeekOpenAICompatibleProvider, + ModelScopeOpenAICompatibleProvider, OpenRouterOpenAICompatibleProvider, type OpenAICompatibleProvider, DefaultOpenAICompatibleProvider, @@ -78,6 +79,14 @@ export function determineProvider( ); } + // Check for ModelScope provider + if (ModelScopeOpenAICompatibleProvider.isModelScopeProvider(config)) { + return new ModelScopeOpenAICompatibleProvider( + contentGeneratorConfig, + cliConfig, + ); + } + // Default provider for standard OpenAI-compatible APIs return new DefaultOpenAICompatibleProvider(contentGeneratorConfig, cliConfig); } diff --git a/packages/core/src/core/openaiContentGenerator/provider/index.ts b/packages/core/src/core/openaiContentGenerator/provider/index.ts index 9886b70f..cb33834d 100644 --- a/packages/core/src/core/openaiContentGenerator/provider/index.ts +++ b/packages/core/src/core/openaiContentGenerator/provider/index.ts @@ -1,3 +1,4 @@ +export { ModelScopeOpenAICompatibleProvider } from './modelscope.js'; export { DashScopeOpenAICompatibleProvider } from './dashscope.js'; export { DeepSeekOpenAICompatibleProvider } from './deepseek.js'; export { OpenRouterOpenAICompatibleProvider } from './openrouter.js'; diff --git a/packages/core/src/core/openaiContentGenerator/provider/modelscope.test.ts b/packages/core/src/core/openaiContentGenerator/provider/modelscope.test.ts new file mode 100644 index 00000000..da5a71a8 --- /dev/null +++ b/packages/core/src/core/openaiContentGenerator/provider/modelscope.test.ts @@ -0,0 +1,96 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type OpenAI from 'openai'; +import { ModelScopeOpenAICompatibleProvider } from './modelscope.js'; +import type { Config } from '../../../config/config.js'; +import type { ContentGeneratorConfig } from '../../contentGenerator.js'; + +vi.mock('openai'); + +describe('ModelScopeOpenAICompatibleProvider', () => { + let provider: ModelScopeOpenAICompatibleProvider; + let mockContentGeneratorConfig: ContentGeneratorConfig; + let mockCliConfig: Config; + + beforeEach(() => { + mockContentGeneratorConfig = { + apiKey: 'test-api-key', + baseUrl: 'https://api.modelscope.cn/v1', + model: 'qwen-max', + } as ContentGeneratorConfig; + + mockCliConfig = { + getCliVersion: vi.fn().mockReturnValue('1.0.0'), + } as unknown as Config; + + provider = new ModelScopeOpenAICompatibleProvider( + mockContentGeneratorConfig, + mockCliConfig, + ); + }); + + describe('isModelScopeProvider', () => { + it('should return true if baseUrl includes "modelscope"', () => { + const config = { baseUrl: 'https://api.modelscope.cn/v1' }; + expect( + ModelScopeOpenAICompatibleProvider.isModelScopeProvider( + config as ContentGeneratorConfig, + ), + ).toBe(true); + }); + + it('should return false if baseUrl does not include "modelscope"', () => { + const config = { baseUrl: 'https://api.openai.com/v1' }; + expect( + ModelScopeOpenAICompatibleProvider.isModelScopeProvider( + config as ContentGeneratorConfig, + ), + ).toBe(false); + }); + }); + + describe('buildRequest', () => { + it('should remove stream_options when stream is false', () => { + const originalRequest: OpenAI.Chat.ChatCompletionCreateParams = { + model: 'qwen-max', + messages: [{ role: 'user', content: 'Hello!' }], + stream: false, + stream_options: { include_usage: true }, + }; + + const result = provider.buildRequest(originalRequest, 'prompt-id'); + + expect(result).not.toHaveProperty('stream_options'); + }); + + it('should keep stream_options when stream is true', () => { + const originalRequest: OpenAI.Chat.ChatCompletionCreateParams = { + model: 'qwen-max', + messages: [{ role: 'user', content: 'Hello!' }], + stream: true, + stream_options: { include_usage: true }, + }; + + const result = provider.buildRequest(originalRequest, 'prompt-id'); + + expect(result).toHaveProperty('stream_options'); + }); + + it('should handle requests without stream_options', () => { + const originalRequest: OpenAI.Chat.ChatCompletionCreateParams = { + model: 'qwen-max', + messages: [{ role: 'user', content: 'Hello!' }], + stream: false, + }; + + const result = provider.buildRequest(originalRequest, 'prompt-id'); + + expect(result).not.toHaveProperty('stream_options'); + }); + }); +}); diff --git a/packages/core/src/core/openaiContentGenerator/provider/modelscope.ts b/packages/core/src/core/openaiContentGenerator/provider/modelscope.ts new file mode 100644 index 00000000..1afb2d03 --- /dev/null +++ b/packages/core/src/core/openaiContentGenerator/provider/modelscope.ts @@ -0,0 +1,32 @@ +import type OpenAI from 'openai'; +import { DefaultOpenAICompatibleProvider } from './default.js'; +import type { ContentGeneratorConfig } from '../../contentGenerator.js'; + +/** + * Provider for ModelScope API + */ +export class ModelScopeOpenAICompatibleProvider extends DefaultOpenAICompatibleProvider { + /** + * Checks if the configuration is for ModelScope. + */ + static isModelScopeProvider(config: ContentGeneratorConfig): boolean { + return !!config.baseUrl?.includes('modelscope'); + } + + /** + * ModelScope does not support `stream_options` when `stream` is false. + * This method removes `stream_options` if `stream` is not true. + */ + override buildRequest( + request: OpenAI.Chat.ChatCompletionCreateParams, + userPromptId: string, + ): OpenAI.Chat.ChatCompletionCreateParams { + const newRequest = super.buildRequest(request, userPromptId); + if (!newRequest.stream) { + delete (newRequest as OpenAI.Chat.ChatCompletionCreateParamsNonStreaming) + .stream_options; + } + + return newRequest; + } +} From 71646490f1df3d5f26a9fa6122bbf72fd9e06172 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Tue, 18 Nov 2025 19:38:30 +0800 Subject: [PATCH 5/5] Fix: Improve ripgrep binary detection and cross-platform compatibility (#1060) --- packages/core/scripts/postinstall.js | 119 ++++++++------- packages/core/src/tools/ripGrep.test.ts | 15 +- packages/core/src/tools/ripGrep.ts | 23 +-- packages/core/src/utils/ripgrepUtils.test.ts | 150 ++++++++----------- packages/core/src/utils/ripgrepUtils.ts | 100 +++++++------ 5 files changed, 203 insertions(+), 204 deletions(-) diff --git a/packages/core/scripts/postinstall.js b/packages/core/scripts/postinstall.js index 38f6d548..641b89a3 100644 --- a/packages/core/scripts/postinstall.js +++ b/packages/core/scripts/postinstall.js @@ -20,66 +20,81 @@ const vendorDir = path.join(packageRoot, 'vendor', 'ripgrep'); /** * Remove quarantine attribute and set executable permissions on macOS/Linux + * This script never throws errors to avoid blocking npm workflows. */ function setupRipgrepBinaries() { - if (!fs.existsSync(vendorDir)) { - console.log('Vendor directory not found, skipping ripgrep setup'); - return; - } - - const platform = process.platform; - const arch = process.arch; - - // Determine the binary directory based on platform and architecture - let binaryDir; - if (platform === 'darwin' || platform === 'linux') { - const archStr = arch === 'x64' || arch === 'arm64' ? arch : null; - if (archStr) { - binaryDir = path.join(vendorDir, `${archStr}-${platform}`); - } - } else if (platform === 'win32') { - // Windows doesn't need these fixes - return; - } - - if (!binaryDir || !fs.existsSync(binaryDir)) { - console.log( - `Binary directory not found for ${platform}-${arch}, skipping ripgrep setup`, - ); - return; - } - - const rgBinary = path.join(binaryDir, 'rg'); - - if (!fs.existsSync(rgBinary)) { - console.log(`Ripgrep binary not found at ${rgBinary}`); - return; - } - try { - // Set executable permissions - fs.chmodSync(rgBinary, 0o755); - console.log(`✓ Set executable permissions on ${rgBinary}`); + if (!fs.existsSync(vendorDir)) { + console.log('ℹ Vendor directory not found, skipping ripgrep setup'); + return; + } - // On macOS, remove quarantine attribute - if (platform === 'darwin') { - try { - execSync(`xattr -d com.apple.quarantine "${rgBinary}"`, { - stdio: 'pipe', - }); - console.log(`✓ Removed quarantine attribute from ${rgBinary}`); - } catch (error) { - // Quarantine attribute might not exist, which is fine - if (error.message && !error.message.includes('No such xattr')) { - console.warn( - `Warning: Could not remove quarantine attribute: ${error.message}`, - ); + const platform = process.platform; + const arch = process.arch; + + // Determine the binary directory based on platform and architecture + let binaryDir; + if (platform === 'darwin' || platform === 'linux') { + const archStr = arch === 'x64' || arch === 'arm64' ? arch : null; + if (archStr) { + binaryDir = path.join(vendorDir, `${archStr}-${platform}`); + } + } else if (platform === 'win32') { + // Windows doesn't need these fixes + console.log('ℹ Windows detected, skipping ripgrep setup'); + return; + } + + if (!binaryDir || !fs.existsSync(binaryDir)) { + console.log( + `ℹ Binary directory not found for ${platform}-${arch}, skipping ripgrep setup`, + ); + return; + } + + const rgBinary = path.join(binaryDir, 'rg'); + + if (!fs.existsSync(rgBinary)) { + console.log(`ℹ Ripgrep binary not found at ${rgBinary}, skipping setup`); + return; + } + + try { + // Set executable permissions + fs.chmodSync(rgBinary, 0o755); + console.log(`✓ Set executable permissions on ${rgBinary}`); + + // On macOS, remove quarantine attribute + if (platform === 'darwin') { + try { + execSync(`xattr -d com.apple.quarantine "${rgBinary}"`, { + stdio: 'pipe', + }); + console.log(`✓ Removed quarantine attribute from ${rgBinary}`); + } catch { + // Quarantine attribute might not exist, which is fine + console.log('ℹ Quarantine attribute not present or already removed'); } } + } catch (error) { + console.log( + `⚠ Could not complete ripgrep setup: ${error.message || 'Unknown error'}`, + ); + console.log(' This is not critical - ripgrep may still work correctly'); } } catch (error) { - console.error(`Error setting up ripgrep binary: ${error.message}`); + console.log( + `⚠ Ripgrep setup encountered an issue: ${error.message || 'Unknown error'}`, + ); + console.log(' Continuing anyway - this should not affect functionality'); } } -setupRipgrepBinaries(); +// Wrap the entire execution to ensure no errors escape to npm +try { + setupRipgrepBinaries(); +} catch { + // Last resort catch - never let errors block npm + console.log('⚠ Postinstall script encountered an unexpected error'); + console.log(' This will not affect the installation'); +} diff --git a/packages/core/src/tools/ripGrep.test.ts b/packages/core/src/tools/ripGrep.test.ts index a2f813f4..1b7dfe2d 100644 --- a/packages/core/src/tools/ripGrep.test.ts +++ b/packages/core/src/tools/ripGrep.test.ts @@ -22,12 +22,12 @@ import type { Config } from '../config/config.js'; import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; import type { ChildProcess } from 'node:child_process'; import { spawn } from 'node:child_process'; -import { ensureRipgrepPath } from '../utils/ripgrepUtils.js'; +import { getRipgrepCommand } from '../utils/ripgrepUtils.js'; import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js'; // Mock ripgrepUtils vi.mock('../utils/ripgrepUtils.js', () => ({ - ensureRipgrepPath: vi.fn(), + getRipgrepCommand: vi.fn(), })); // Mock child_process for ripgrep calls @@ -109,7 +109,7 @@ describe('RipGrepTool', () => { beforeEach(async () => { vi.clearAllMocks(); - (ensureRipgrepPath as Mock).mockResolvedValue('/mock/path/to/rg'); + (getRipgrepCommand as Mock).mockResolvedValue('/mock/path/to/rg'); mockSpawn.mockReset(); tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'grep-tool-root-')); fileExclusionsMock = { @@ -588,18 +588,15 @@ describe('RipGrepTool', () => { }); it('should throw an error if ripgrep is not available', async () => { - // Make ensureRipgrepBinary throw - (ensureRipgrepPath as Mock).mockRejectedValue( - new Error('Ripgrep binary not found'), - ); + (getRipgrepCommand as Mock).mockResolvedValue(null); const params: RipGrepToolParams = { pattern: 'world' }; const invocation = grepTool.build(params); expect(await invocation.execute(abortSignal)).toStrictEqual({ llmContent: - 'Error during grep search operation: Ripgrep binary not found', - returnDisplay: 'Error: Ripgrep binary not found', + 'Error during grep search operation: ripgrep binary not found.', + returnDisplay: 'Error: ripgrep binary not found.', }); }); }); diff --git a/packages/core/src/tools/ripGrep.ts b/packages/core/src/tools/ripGrep.ts index 80273f31..402a5d3c 100644 --- a/packages/core/src/tools/ripGrep.ts +++ b/packages/core/src/tools/ripGrep.ts @@ -6,7 +6,6 @@ import fs from 'node:fs'; import path from 'node:path'; -import { EOL } from 'node:os'; import { spawn } from 'node:child_process'; import type { ToolInvocation, ToolResult } from './tools.js'; import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; @@ -14,7 +13,7 @@ import { ToolNames } from './tool-names.js'; import { resolveAndValidatePath } from '../utils/paths.js'; import { getErrorMessage } from '../utils/errors.js'; import type { Config } from '../config/config.js'; -import { ensureRipgrepPath } from '../utils/ripgrepUtils.js'; +import { getRipgrepCommand } from '../utils/ripgrepUtils.js'; import { SchemaValidator } from '../utils/schemaValidator.js'; import type { FileFilteringOptions } from '../config/constants.js'; import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js'; @@ -88,7 +87,7 @@ class GrepToolInvocation extends BaseToolInvocation< } // Split into lines and count total matches - const allLines = rawOutput.split(EOL).filter((line) => line.trim()); + const allLines = rawOutput.split('\n').filter((line) => line.trim()); const totalMatches = allLines.length; const matchTerm = totalMatches === 1 ? 'match' : 'matches'; @@ -159,7 +158,7 @@ class GrepToolInvocation extends BaseToolInvocation< returnDisplay: displayMessage, }; } catch (error) { - console.error(`Error during GrepLogic execution: ${error}`); + console.error(`Error during ripgrep search operation: ${error}`); const errorMessage = getErrorMessage(error); return { llmContent: `Error during grep search operation: ${errorMessage}`, @@ -210,11 +209,15 @@ class GrepToolInvocation extends BaseToolInvocation< rgArgs.push(absolutePath); try { - const rgPath = this.config.getUseBuiltinRipgrep() - ? await ensureRipgrepPath() - : 'rg'; + const rgCommand = await getRipgrepCommand( + this.config.getUseBuiltinRipgrep(), + ); + if (!rgCommand) { + throw new Error('ripgrep binary not found.'); + } + const output = await new Promise((resolve, reject) => { - const child = spawn(rgPath, rgArgs, { + const child = spawn(rgCommand, rgArgs, { windowsHide: true, }); @@ -234,7 +237,7 @@ class GrepToolInvocation extends BaseToolInvocation< child.on('error', (err) => { options.signal.removeEventListener('abort', cleanup); - reject(new Error(`Failed to start ripgrep: ${err.message}.`)); + reject(new Error(`failed to start ripgrep: ${err.message}.`)); }); child.on('close', (code) => { @@ -256,7 +259,7 @@ class GrepToolInvocation extends BaseToolInvocation< return output; } catch (error: unknown) { - console.error(`GrepLogic: ripgrep failed: ${getErrorMessage(error)}`); + console.error(`Ripgrep failed: ${getErrorMessage(error)}`); throw error; } } diff --git a/packages/core/src/utils/ripgrepUtils.test.ts b/packages/core/src/utils/ripgrepUtils.test.ts index 59180aa8..47bba8a5 100644 --- a/packages/core/src/utils/ripgrepUtils.test.ts +++ b/packages/core/src/utils/ripgrepUtils.test.ts @@ -7,8 +7,8 @@ import { describe, it, expect, beforeEach, vi, type Mock } from 'vitest'; import { canUseRipgrep, - ensureRipgrepPath, - getRipgrepPath, + getRipgrepCommand, + getBuiltinRipgrep, } from './ripgrepUtils.js'; import { fileExists } from './fileUtils.js'; import path from 'node:path'; @@ -27,7 +27,7 @@ describe('ripgrepUtils', () => { vi.clearAllMocks(); }); - describe('getRipgrepPath', () => { + describe('getBulltinRipgrepPath', () => { it('should return path with .exe extension on Windows', () => { const originalPlatform = process.platform; const originalArch = process.arch; @@ -36,7 +36,7 @@ describe('ripgrepUtils', () => { Object.defineProperty(process, 'platform', { value: 'win32' }); Object.defineProperty(process, 'arch', { value: 'x64' }); - const rgPath = getRipgrepPath(); + const rgPath = getBuiltinRipgrep(); expect(rgPath).toContain('x64-win32'); expect(rgPath).toContain('rg.exe'); @@ -55,7 +55,7 @@ describe('ripgrepUtils', () => { Object.defineProperty(process, 'platform', { value: 'darwin' }); Object.defineProperty(process, 'arch', { value: 'arm64' }); - const rgPath = getRipgrepPath(); + const rgPath = getBuiltinRipgrep(); expect(rgPath).toContain('arm64-darwin'); expect(rgPath).toContain('rg'); @@ -75,7 +75,7 @@ describe('ripgrepUtils', () => { Object.defineProperty(process, 'platform', { value: 'linux' }); Object.defineProperty(process, 'arch', { value: 'x64' }); - const rgPath = getRipgrepPath(); + const rgPath = getBuiltinRipgrep(); expect(rgPath).toContain('x64-linux'); expect(rgPath).toContain('rg'); @@ -87,7 +87,7 @@ describe('ripgrepUtils', () => { Object.defineProperty(process, 'arch', { value: originalArch }); }); - it('should throw error for unsupported platform', () => { + it('should return null for unsupported platform', () => { const originalPlatform = process.platform; const originalArch = process.arch; @@ -95,14 +95,14 @@ describe('ripgrepUtils', () => { Object.defineProperty(process, 'platform', { value: 'freebsd' }); Object.defineProperty(process, 'arch', { value: 'x64' }); - expect(() => getRipgrepPath()).toThrow('Unsupported platform: freebsd'); + expect(getBuiltinRipgrep()).toBeNull(); // Restore original values Object.defineProperty(process, 'platform', { value: originalPlatform }); Object.defineProperty(process, 'arch', { value: originalArch }); }); - it('should throw error for unsupported architecture', () => { + it('should return null for unsupported architecture', () => { const originalPlatform = process.platform; const originalArch = process.arch; @@ -110,7 +110,7 @@ describe('ripgrepUtils', () => { Object.defineProperty(process, 'platform', { value: 'darwin' }); Object.defineProperty(process, 'arch', { value: 'ia32' }); - expect(() => getRipgrepPath()).toThrow('Unsupported architecture: ia32'); + expect(getBuiltinRipgrep()).toBeNull(); // Restore original values Object.defineProperty(process, 'platform', { value: originalPlatform }); @@ -136,7 +136,7 @@ describe('ripgrepUtils', () => { Object.defineProperty(process, 'platform', { value: platform }); Object.defineProperty(process, 'arch', { value: arch }); - const rgPath = getRipgrepPath(); + const rgPath = getBuiltinRipgrep(); const binaryName = platform === 'win32' ? 'rg.exe' : 'rg'; const expectedPathSegment = path.join( `${arch}-${platform}`, @@ -169,107 +169,77 @@ describe('ripgrepUtils', () => { expect(result).toBe(true); expect(fileExists).toHaveBeenCalledOnce(); }); - - it('should fall back to system rg if bundled ripgrep binary does not exist', async () => { - (fileExists as Mock).mockResolvedValue(false); - // When useBuiltin is true but bundled binary doesn't exist, - // it should fall back to checking system rg (which will spawn a process) - // In this test environment, system rg is likely available, so result should be true - // unless spawn fails - - const result = await canUseRipgrep(); - - // The test may pass or fail depending on system rg availability - // Just verify that fileExists was called to check bundled binary first - expect(fileExists).toHaveBeenCalledOnce(); - // Result depends on whether system rg is installed - expect(typeof result).toBe('boolean'); - }); - - // Note: Tests for system ripgrep detection (useBuiltin=false) would require mocking - // the child_process spawn function, which is complex in ESM. These cases are tested - // indirectly through integration tests. - - it('should return false if platform is unsupported', async () => { - const originalPlatform = process.platform; - - // Mock unsupported platform - Object.defineProperty(process, 'platform', { value: 'aix' }); - - const result = await canUseRipgrep(); - - expect(result).toBe(false); - expect(fileExists).not.toHaveBeenCalled(); - - // Restore original value - Object.defineProperty(process, 'platform', { value: originalPlatform }); - }); - - it('should return false if architecture is unsupported', async () => { - const originalArch = process.arch; - - // Mock unsupported architecture - Object.defineProperty(process, 'arch', { value: 's390x' }); - - const result = await canUseRipgrep(); - - expect(result).toBe(false); - expect(fileExists).not.toHaveBeenCalled(); - - // Restore original value - Object.defineProperty(process, 'arch', { value: originalArch }); - }); }); - describe('ensureRipgrepBinary', () => { - it('should return ripgrep path if binary exists', async () => { + describe('ensureRipgrepPath', () => { + it('should return bundled ripgrep path if binary exists (useBuiltin=true)', async () => { (fileExists as Mock).mockResolvedValue(true); - const rgPath = await ensureRipgrepPath(); + const rgPath = await getRipgrepCommand(true); expect(rgPath).toBeDefined(); expect(rgPath).toContain('rg'); + expect(rgPath).not.toBe('rg'); // Should be full path, not just 'rg' expect(fileExists).toHaveBeenCalledOnce(); expect(fileExists).toHaveBeenCalledWith(rgPath); }); - it('should throw error if binary does not exist', async () => { - (fileExists as Mock).mockResolvedValue(false); + it('should return bundled ripgrep path if binary exists (default)', async () => { + (fileExists as Mock).mockResolvedValue(true); - await expect(ensureRipgrepPath()).rejects.toThrow( - /Ripgrep binary not found/, - ); - await expect(ensureRipgrepPath()).rejects.toThrow(/Platform:/); - await expect(ensureRipgrepPath()).rejects.toThrow(/Architecture:/); + const rgPath = await getRipgrepCommand(); - expect(fileExists).toHaveBeenCalled(); + expect(rgPath).toBeDefined(); + expect(rgPath).toContain('rg'); + expect(fileExists).toHaveBeenCalledOnce(); }); - it('should throw error with correct path information', async () => { + it('should fall back to system rg if bundled binary does not exist', async () => { (fileExists as Mock).mockResolvedValue(false); + // When useBuiltin is true but bundled binary doesn't exist, + // it should fall back to checking system rg + // The test result depends on whether system rg is actually available + + const rgPath = await getRipgrepCommand(true); + + expect(fileExists).toHaveBeenCalledOnce(); + // If system rg is available, it should return 'rg' (or 'rg.exe' on Windows) + // This test will pass if system ripgrep is installed + expect(rgPath).toBeDefined(); + }); + + it('should use system rg when useBuiltin=false', async () => { + // When useBuiltin is false, should skip bundled check and go straight to system rg + const rgPath = await getRipgrepCommand(false); + + // Should not check for bundled binary + expect(fileExists).not.toHaveBeenCalled(); + // If system rg is available, it should return 'rg' (or 'rg.exe' on Windows) + expect(rgPath).toBeDefined(); + }); + + it('should throw error if neither bundled nor system ripgrep is available', async () => { + // This test only makes sense in an environment where system rg is not installed + // We'll skip this test in CI/local environments where rg might be available + // Instead, we test the error message format + const originalPlatform = process.platform; + + // Use an unsupported platform to trigger the error path + Object.defineProperty(process, 'platform', { value: 'freebsd' }); try { - await ensureRipgrepPath(); - // Should not reach here - expect(true).toBe(false); + await getRipgrepCommand(); + // If we get here without error, system rg was available, which is fine } catch (error) { expect(error).toBeInstanceOf(Error); const errorMessage = (error as Error).message; - expect(errorMessage).toContain('Ripgrep binary not found at'); - expect(errorMessage).toContain(process.platform); - expect(errorMessage).toContain(process.arch); + // Should contain helpful error information + expect( + errorMessage.includes('Ripgrep binary not found') || + errorMessage.includes('Failed to locate ripgrep') || + errorMessage.includes('Unsupported platform'), + ).toBe(true); } - }); - - it('should throw error if platform is unsupported', async () => { - const originalPlatform = process.platform; - - // Mock unsupported platform - Object.defineProperty(process, 'platform', { value: 'openbsd' }); - - await expect(ensureRipgrepPath()).rejects.toThrow( - 'Unsupported platform: openbsd', - ); // Restore original value Object.defineProperty(process, 'platform', { value: originalPlatform }); diff --git a/packages/core/src/utils/ripgrepUtils.ts b/packages/core/src/utils/ripgrepUtils.ts index 467bf2ca..c6d795a3 100644 --- a/packages/core/src/utils/ripgrepUtils.ts +++ b/packages/core/src/utils/ripgrepUtils.ts @@ -18,37 +18,42 @@ type Architecture = 'x64' | 'arm64'; /** * Maps process.platform values to vendor directory names */ -function getPlatformString(platform: string): Platform { +function getPlatformString(platform: string): Platform | undefined { switch (platform) { case 'darwin': case 'linux': case 'win32': return platform; default: - throw new Error(`Unsupported platform: ${platform}`); + return undefined; } } /** * Maps process.arch values to vendor directory names */ -function getArchitectureString(arch: string): Architecture { +function getArchitectureString(arch: string): Architecture | undefined { switch (arch) { case 'x64': case 'arm64': return arch; default: - throw new Error(`Unsupported architecture: ${arch}`); + return undefined; } } /** * Returns the path to the bundled ripgrep binary for the current platform + * @returns The path to the bundled ripgrep binary, or null if not available */ -export function getRipgrepPath(): string { +export function getBuiltinRipgrep(): string | null { const platform = getPlatformString(process.platform); const arch = getArchitectureString(process.arch); + if (!platform || !arch) { + return null; + } + // Binary name includes .exe on Windows const binaryName = platform === 'win32' ? 'rg.exe' : 'rg'; @@ -83,6 +88,51 @@ export function getRipgrepPath(): string { return vendorPath; } +/** + * Checks if system ripgrep is available and returns the command to use + * @returns The ripgrep command ('rg' or 'rg.exe') if available, or null if not found + */ +export async function getSystemRipgrep(): Promise { + try { + const { spawn } = await import('node:child_process'); + const rgCommand = process.platform === 'win32' ? 'rg.exe' : 'rg'; + const isAvailable = await new Promise((resolve) => { + const proc = spawn(rgCommand, ['--version']); + proc.on('error', () => resolve(false)); + proc.on('exit', (code) => resolve(code === 0)); + }); + return isAvailable ? rgCommand : null; + } catch (_error) { + return null; + } +} + +/** + * Checks if ripgrep binary exists and returns its path + * @param useBuiltin If true, tries bundled ripgrep first, then falls back to system ripgrep. + * If false, only checks for system ripgrep. + * @returns The path to ripgrep binary ('rg' or 'rg.exe' for system ripgrep, or full path for bundled), or null if not available + */ +export async function getRipgrepCommand( + useBuiltin: boolean = true, +): Promise { + try { + if (useBuiltin) { + // Try bundled ripgrep first + const rgPath = getBuiltinRipgrep(); + if (rgPath && (await fileExists(rgPath))) { + return rgPath; + } + // Fallback to system rg if bundled binary is not available + } + + // Check for system ripgrep + return await getSystemRipgrep(); + } catch (_error) { + return null; + } +} + /** * Checks if ripgrep binary is available * @param useBuiltin If true, tries bundled ripgrep first, then falls back to system ripgrep. @@ -91,42 +141,6 @@ export function getRipgrepPath(): string { export async function canUseRipgrep( useBuiltin: boolean = true, ): Promise { - try { - if (useBuiltin) { - // Try bundled ripgrep first - const rgPath = getRipgrepPath(); - if (await fileExists(rgPath)) { - return true; - } - // Fallback to system rg if bundled binary is not available - } - - // Check for system ripgrep by trying to spawn 'rg --version' - const { spawn } = await import('node:child_process'); - return await new Promise((resolve) => { - const proc = spawn('rg', ['--version']); - proc.on('error', () => resolve(false)); - proc.on('exit', (code) => resolve(code === 0)); - }); - } catch (_error) { - // Unsupported platform/arch or other error - return false; - } -} - -/** - * Ensures ripgrep binary exists and returns its path - * @throws Error if ripgrep binary is not available - */ -export async function ensureRipgrepPath(): Promise { - const rgPath = getRipgrepPath(); - - if (!(await fileExists(rgPath))) { - throw new Error( - `Ripgrep binary not found at ${rgPath}. ` + - `Platform: ${process.platform}, Architecture: ${process.arch}`, - ); - } - - return rgPath; + const rgPath = await getRipgrepCommand(useBuiltin); + return rgPath !== null; }