From 8c317755736b0203899792f78998c43bc550ce1d Mon Sep 17 00:00:00 2001 From: LaZzyMan Date: Tue, 24 Mar 2026 16:32:51 +0800 Subject: [PATCH 1/2] fix search tool for multi dirs --- packages/core/src/tools/glob.ts | 143 +++++++++++++++++++---------- packages/core/src/tools/grep.ts | 64 +++++++++---- packages/core/src/tools/ripGrep.ts | 61 ++++++++---- 3 files changed, 182 insertions(+), 86 deletions(-) diff --git a/packages/core/src/tools/glob.ts b/packages/core/src/tools/glob.ts index 12a29922a2..44edae4e64 100644 --- a/packages/core/src/tools/glob.ts +++ b/packages/core/src/tools/glob.ts @@ -119,61 +119,104 @@ class GlobToolInvocation extends BaseToolInvocation< return 'ask'; } + /** + * Runs glob search in a single directory and returns filtered entries. + */ + private async globInDirectory( + searchDir: string, + pattern: string, + signal: AbortSignal, + ): Promise { + let effectivePattern = pattern; + const fullPath = path.join(searchDir, effectivePattern); + if (fs.existsSync(fullPath)) { + effectivePattern = escape(effectivePattern); + } + + const entries = (await glob(effectivePattern, { + cwd: searchDir, + withFileTypes: true, + nodir: true, + stat: true, + nocase: true, + dot: true, + follow: false, + signal, + })) as GlobPath[]; + + // Filter using paths relative to the search directory so that + // .gitignore / .qwenignore patterns match correctly regardless of + // which workspace directory the file belongs to. + const relativePaths = entries.map((p) => + path.relative(searchDir, p.fullpath()), + ); + + const { filteredPaths } = this.fileService.filterFilesWithReport( + relativePaths, + this.getFileFilteringOptions(), + ); + + const normalizePathForComparison = (p: string) => + process.platform === 'win32' || process.platform === 'darwin' + ? p.toLowerCase() + : p; + + const filteredAbsolutePaths = new Set( + filteredPaths.map((p) => + normalizePathForComparison(path.resolve(searchDir, p)), + ), + ); + + return entries.filter((entry) => + filteredAbsolutePaths.has(normalizePathForComparison(entry.fullpath())), + ); + } + async execute(signal: AbortSignal): Promise { try { - // Default to target directory if no path is provided - const searchDirAbs = resolveAndValidatePath( - this.config, - this.params.path, - { allowExternalPaths: true }, - ); - const searchLocationDescription = this.params.path - ? `within ${searchDirAbs}` - : `in the workspace directory`; - - // Collect entries from the search directory - let pattern = this.params.pattern; - const fullPath = path.join(searchDirAbs, pattern); - if (fs.existsSync(fullPath)) { - pattern = escape(pattern); + // Determine which directories to search + const searchDirs: string[] = []; + let searchLocationDescription: string; + + if (this.params.path) { + // User specified a path — search only that directory + const searchDirAbs = resolveAndValidatePath( + this.config, + this.params.path, + { allowExternalPaths: true }, + ); + searchDirs.push(searchDirAbs); + searchLocationDescription = `within ${searchDirAbs}`; + } else { + // No path specified — search all workspace directories + const workspaceDirs = this.config + .getWorkspaceContext() + .getDirectories(); + searchDirs.push(...workspaceDirs); + searchLocationDescription = + workspaceDirs.length > 1 + ? `across ${workspaceDirs.length} workspace directories` + : `in the workspace directory`; } - const allEntries = (await glob(pattern, { - cwd: searchDirAbs, - withFileTypes: true, - nodir: true, - stat: true, - nocase: true, - dot: true, - follow: false, - signal, - })) as GlobPath[]; - - const relativePaths = allEntries.map((p) => - path.relative(this.config.getTargetDir(), p.fullpath()), - ); - - const { filteredPaths } = this.fileService.filterFilesWithReport( - relativePaths, - this.getFileFilteringOptions(), - ); - - const normalizePathForComparison = (p: string) => - process.platform === 'win32' || process.platform === 'darwin' - ? p.toLowerCase() - : p; - - const filteredAbsolutePaths = new Set( - filteredPaths.map((p) => - normalizePathForComparison( - path.resolve(this.config.getTargetDir(), p), - ), - ), - ); + // Collect entries from all search directories + const pattern = this.params.pattern; + const allFilteredEntries: GlobPath[] = []; + const seenPaths = new Set(); + + for (const searchDir of searchDirs) { + const entries = await this.globInDirectory(searchDir, pattern, signal); + for (const entry of entries) { + // Deduplicate entries that might appear in overlapping directories + const normalized = entry.fullpath(); + if (!seenPaths.has(normalized)) { + seenPaths.add(normalized); + allFilteredEntries.push(entry); + } + } + } - const filteredEntries = allEntries.filter((entry) => - filteredAbsolutePaths.has(normalizePathForComparison(entry.fullpath())), - ); + const filteredEntries = allFilteredEntries; if (!filteredEntries || filteredEntries.length === 0) { return { diff --git a/packages/core/src/tools/grep.ts b/packages/core/src/tools/grep.ts index e0df29140c..6e16348d95 100644 --- a/packages/core/src/tools/grep.ts +++ b/packages/core/src/tools/grep.ts @@ -95,26 +95,52 @@ class GrepToolInvocation extends BaseToolInvocation< async execute(signal: AbortSignal): Promise { try { - // Default to target directory if no path is provided - const searchDirAbs = resolveAndValidatePath( - this.config, - this.params.path, - { allowExternalPaths: true }, - ); - const searchDirDisplay = this.params.path || '.'; - - // Perform grep search - const rawMatches = await this.performGrepSearch({ - pattern: this.params.pattern, - path: searchDirAbs, - glob: this.params.glob, - signal, - }); + // Determine which directories to search + const searchDirs: string[] = []; + let searchLocationDescription: string; + + if (this.params.path) { + // User specified a path — search only that directory + const searchDirAbs = resolveAndValidatePath( + this.config, + this.params.path, + { allowExternalPaths: true }, + ); + searchDirs.push(searchDirAbs); + searchLocationDescription = `in path "${this.params.path}"`; + } else { + // No path specified — search all workspace directories + const workspaceDirs = this.config + .getWorkspaceContext() + .getDirectories(); + searchDirs.push(...workspaceDirs); + searchLocationDescription = + workspaceDirs.length > 1 + ? `across ${workspaceDirs.length} workspace directories` + : `in the workspace directory`; + } - // Build search description - const searchLocationDescription = this.params.path - ? `in path "${searchDirDisplay}"` - : `in the workspace directory`; + // Perform grep search across all directories + const rawMatches: GrepMatch[] = []; + for (const searchDir of searchDirs) { + const matches = await this.performGrepSearch({ + pattern: this.params.pattern, + path: searchDir, + glob: this.params.glob, + signal, + }); + // When searching multiple directories, convert relative file paths + // to absolute paths so results from different directories are + // unambiguous. + if (searchDirs.length > 1) { + for (const match of matches) { + if (!path.isAbsolute(match.filePath)) { + match.filePath = path.resolve(searchDir, match.filePath); + } + } + } + rawMatches.push(...matches); + } const filterDescription = this.params.glob ? ` (filter: "${this.params.glob}")` diff --git a/packages/core/src/tools/ripGrep.ts b/packages/core/src/tools/ripGrep.ts index 4419e07967..19a98af800 100644 --- a/packages/core/src/tools/ripGrep.ts +++ b/packages/core/src/tools/ripGrep.ts @@ -58,17 +58,32 @@ class GrepToolInvocation extends BaseToolInvocation< async execute(signal: AbortSignal): Promise { try { - const searchDirAbs = resolveAndValidatePath( - this.config, - this.params.path, - { allowFiles: true }, - ); - const searchDirDisplay = this.params.path || '.'; + // Determine which paths to search + const searchPaths: string[] = []; + let searchDirDisplay: string; + + if (this.params.path) { + // User specified a path — search only that path + const searchDirAbs = resolveAndValidatePath( + this.config, + this.params.path, + { allowFiles: true }, + ); + searchPaths.push(searchDirAbs); + searchDirDisplay = this.params.path; + } else { + // No path specified — search all workspace directories + const workspaceDirs = this.config + .getWorkspaceContext() + .getDirectories(); + searchPaths.push(...workspaceDirs); + searchDirDisplay = '.'; + } // Get raw ripgrep output const rawOutput = await this.performRipgrepSearch({ pattern: this.params.pattern, - path: searchDirAbs, + paths: searchPaths, glob: this.params.glob, signal, }); @@ -76,7 +91,9 @@ class GrepToolInvocation extends BaseToolInvocation< // Build search description const searchLocationDescription = this.params.path ? `in path "${searchDirDisplay}"` - : `in the workspace directory`; + : searchPaths.length > 1 + ? `across ${searchPaths.length} workspace directories` + : `in the workspace directory`; const filterDescription = this.params.glob ? ` (filter: "${this.params.glob}")` @@ -171,11 +188,11 @@ class GrepToolInvocation extends BaseToolInvocation< private async performRipgrepSearch(options: { pattern: string; - path: string; // Can be a file or directory + paths: string[]; // Can be files or directories glob?: string; signal: AbortSignal; }): Promise { - const { pattern, path: absolutePath, glob } = options; + const { pattern, paths, glob } = options; const rgArgs: string[] = [ '--line-number', @@ -193,12 +210,21 @@ class GrepToolInvocation extends BaseToolInvocation< } if (filteringOptions.respectQwenIgnore) { - const qwenIgnorePath = path.join( - this.config.getTargetDir(), - '.qwenignore', - ); - if (fs.existsSync(qwenIgnorePath)) { - rgArgs.push('--ignore-file', qwenIgnorePath); + // Load .qwenignore from each workspace directory, not just the primary one + const seenIgnoreFiles = new Set(); + for (const searchPath of paths) { + const dir = + fs.existsSync(searchPath) && fs.statSync(searchPath).isDirectory() + ? searchPath + : path.dirname(searchPath); + const qwenIgnorePath = path.join(dir, '.qwenignore'); + if ( + !seenIgnoreFiles.has(qwenIgnorePath) && + fs.existsSync(qwenIgnorePath) + ) { + rgArgs.push('--ignore-file', qwenIgnorePath); + seenIgnoreFiles.add(qwenIgnorePath); + } } } @@ -208,7 +234,8 @@ class GrepToolInvocation extends BaseToolInvocation< } rgArgs.push('--threads', '4'); - rgArgs.push(absolutePath); + // Pass all search paths to ripgrep (it supports multiple paths natively) + rgArgs.push(...paths); const result = await runRipgrep(rgArgs, options.signal); if (result.error && !result.stdout) { From a5a8ec5d67fd763ea7204508d3e27eab45dacd64 Mon Sep 17 00:00:00 2001 From: LaZzyMan Date: Tue, 24 Mar 2026 19:47:07 +0800 Subject: [PATCH 2/2] feat: human-readable permission labels, deny rule feedback, and multi-dir search tests - Add buildHumanReadableRuleLabel() to convert raw permission rules into natural-language descriptions for the 'Always Allow' UI options - Add PermissionManager.findMatchingDenyRule() to surface which deny rule caused a tool to be blocked, improving error messages in coreToolScheduler - Update ToolConfirmationMessage to use friendly labels with i18n support - Add comprehensive tests for new permission features and multi-directory search in glob, grep, and ripGrep tools - Fix integration test for tool-control allowedTools configuration --- packages/cli/src/i18n/locales/de.js | 4 + packages/cli/src/i18n/locales/en.js | 4 + packages/cli/src/i18n/locales/ja.js | 3 + packages/cli/src/i18n/locales/pt.js | 4 + packages/cli/src/i18n/locales/ru.js | 4 + packages/cli/src/i18n/locales/zh.js | 2 + .../messages/ToolConfirmationMessage.tsx | 54 ++++-- packages/core/src/core/coreToolScheduler.ts | 24 ++- .../permissions/permission-manager.test.ts | 172 ++++++++++++++++++ .../src/permissions/permission-manager.ts | 37 ++++ packages/core/src/permissions/rule-parser.ts | 100 ++++++++++ packages/core/src/tools/glob.test.ts | 81 +++++++++ packages/core/src/tools/grep.test.ts | 42 +++++ packages/core/src/tools/ripGrep.test.ts | 110 +++++++++++ 14 files changed, 622 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/i18n/locales/de.js b/packages/cli/src/i18n/locales/de.js index aa4a6d5520..ff700188c6 100644 --- a/packages/cli/src/i18n/locales/de.js +++ b/packages/cli/src/i18n/locales/de.js @@ -1047,7 +1047,11 @@ export default { "Ausführung erlauben von: '{{command}}'?", 'Yes, allow always ...': 'Ja, immer erlauben ...', 'Always allow in this project': 'In diesem Projekt immer erlauben', + 'Always allow {{action}} in this project': + '{{action}} in diesem Projekt immer erlauben', 'Always allow for this user': 'Für diesen Benutzer immer erlauben', + 'Always allow {{action}} for this user': + '{{action}} für diesen Benutzer immer erlauben', 'Yes, and auto-accept edits': 'Ja, und Änderungen automatisch akzeptieren', 'Yes, and manually approve edits': 'Ja, und Änderungen manuell genehmigen', 'No, keep planning (esc)': 'Nein, weiter planen (Esc)', diff --git a/packages/cli/src/i18n/locales/en.js b/packages/cli/src/i18n/locales/en.js index fb4433b2a5..8c37e2f78f 100644 --- a/packages/cli/src/i18n/locales/en.js +++ b/packages/cli/src/i18n/locales/en.js @@ -1103,7 +1103,11 @@ export default { "Allow execution of: '{{command}}'?": "Allow execution of: '{{command}}'?", 'Yes, allow always ...': 'Yes, allow always ...', 'Always allow in this project': 'Always allow in this project', + 'Always allow {{action}} in this project': + 'Always allow {{action}} in this project', 'Always allow for this user': 'Always allow for this user', + 'Always allow {{action}} for this user': + 'Always allow {{action}} for this user', 'Yes, and auto-accept edits': 'Yes, and auto-accept edits', 'Yes, and manually approve edits': 'Yes, and manually approve edits', 'No, keep planning (esc)': 'No, keep planning (esc)', diff --git a/packages/cli/src/i18n/locales/ja.js b/packages/cli/src/i18n/locales/ja.js index b06a6fdef6..677b85a67f 100644 --- a/packages/cli/src/i18n/locales/ja.js +++ b/packages/cli/src/i18n/locales/ja.js @@ -786,7 +786,10 @@ export default { "Allow execution of: '{{command}}'?": "'{{command}}' の実行を許可しますか?", 'Yes, allow always ...': 'はい、常に許可...', 'Always allow in this project': 'このプロジェクトで常に許可', + 'Always allow {{action}} in this project': + 'このプロジェクトで{{action}}を常に許可', 'Always allow for this user': 'このユーザーに常に許可', + 'Always allow {{action}} for this user': 'このユーザーに{{action}}を常に許可', 'Yes, and auto-accept edits': 'はい、編集を自動承認', 'Yes, and manually approve edits': 'はい、編集を手動承認', 'No, keep planning (esc)': 'いいえ、計画を続ける (Esc)', diff --git a/packages/cli/src/i18n/locales/pt.js b/packages/cli/src/i18n/locales/pt.js index b2240877b3..1a0b26a39e 100644 --- a/packages/cli/src/i18n/locales/pt.js +++ b/packages/cli/src/i18n/locales/pt.js @@ -1054,7 +1054,11 @@ export default { "Permitir a execução de: '{{command}}'?", 'Yes, allow always ...': 'Sim, permitir sempre ...', 'Always allow in this project': 'Sempre permitir neste projeto', + 'Always allow {{action}} in this project': + 'Sempre permitir {{action}} neste projeto', 'Always allow for this user': 'Sempre permitir para este usuário', + 'Always allow {{action}} for this user': + 'Sempre permitir {{action}} para este usuário', 'Yes, and auto-accept edits': 'Sim, e aceitar edições automaticamente', 'Yes, and manually approve edits': 'Sim, e aprovar edições manualmente', 'No, keep planning (esc)': 'Não, continuar planejando (esc)', diff --git a/packages/cli/src/i18n/locales/ru.js b/packages/cli/src/i18n/locales/ru.js index c3ae5953a5..49226706cd 100644 --- a/packages/cli/src/i18n/locales/ru.js +++ b/packages/cli/src/i18n/locales/ru.js @@ -979,7 +979,11 @@ export default { "Allow execution of: '{{command}}'?": "Разрешить выполнение: '{{command}}'?", 'Yes, allow always ...': 'Да, всегда разрешать ...', 'Always allow in this project': 'Всегда разрешать в этом проекте', + 'Always allow {{action}} in this project': + 'Всегда разрешать {{action}} в этом проекте', 'Always allow for this user': 'Всегда разрешать для этого пользователя', + 'Always allow {{action}} for this user': + 'Всегда разрешать {{action}} для этого пользователя', 'Yes, and auto-accept edits': 'Да, и автоматически принимать правки', 'Yes, and manually approve edits': 'Да, и вручную подтверждать правки', 'No, keep planning (esc)': 'Нет, продолжить планирование (esc)', diff --git a/packages/cli/src/i18n/locales/zh.js b/packages/cli/src/i18n/locales/zh.js index d22fe9b262..f2428fd23f 100644 --- a/packages/cli/src/i18n/locales/zh.js +++ b/packages/cli/src/i18n/locales/zh.js @@ -1044,7 +1044,9 @@ export default { "Allow execution of: '{{command}}'?": "允许执行：'{{command}}'？", 'Yes, allow always ...': '是，总是允许 ...', 'Always allow in this project': '在本项目中总是允许', + 'Always allow {{action}} in this project': '在本项目中总是允许{{action}}', 'Always allow for this user': '对该用户总是允许', + 'Always allow {{action}} for this user': '对该用户总是允许{{action}}', 'Yes, and auto-accept edits': '是，并自动接受编辑', 'Yes, and manually approve edits': '是，并手动批准编辑', 'No, keep planning (esc)': '否，继续规划 (esc)', diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 3946b0b05d..e3c9ed1e1d 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -17,7 +17,11 @@ import type { Config, EditorType, } from '@qwen-code/qwen-code-core'; -import { IdeClient, ToolConfirmationOutcome } from '@qwen-code/qwen-code-core'; +import { + IdeClient, + ToolConfirmationOutcome, + buildHumanReadableRuleLabel, +} from '@qwen-code/qwen-code-core'; import type { RadioSelectItem } from '../shared/RadioButtonSelect.js'; import { RadioButtonSelect } from '../shared/RadioButtonSelect.js'; import { MaxSizedBox } from '../shared/MaxSizedBox.js'; @@ -243,16 +247,24 @@ export const ToolConfirmationMessage: React.FC< key: 'Yes, allow once', }); if (isTrustedFolder && !confirmationDetails.hideAlwaysAllow) { - const rulesLabel = executionProps.permissionRules?.length - ? ` [${executionProps.permissionRules.join(', ')}]` + const friendlyLabel = executionProps.permissionRules?.length + ? ` ${buildHumanReadableRuleLabel(executionProps.permissionRules)}` : ''; options.push({ - label: t('Always allow in this project') + rulesLabel, + label: friendlyLabel + ? t('Always allow {{action}} in this project', { + action: friendlyLabel.trim(), + }) + : t('Always allow in this project'), value: ToolConfirmationOutcome.ProceedAlwaysProject, key: 'Always allow in this project', }); options.push({ - label: t('Always allow for this user') + rulesLabel, + label: friendlyLabel + ? t('Always allow {{action}} for this user', { + action: friendlyLabel.trim(), + }) + : t('Always allow for this user'), value: ToolConfirmationOutcome.ProceedAlwaysUser, key: 'Always allow for this user', }); @@ -324,18 +336,26 @@ export const ToolConfirmationMessage: React.FC< key: 'Yes, allow once', }); if (isTrustedFolder && !confirmationDetails.hideAlwaysAllow) { - const rulesLabel = + const friendlyLabel = 'permissionRules' in infoProps && (infoProps as { permissionRules?: string[] }).permissionRules?.length - ? ` [${(infoProps as { permissionRules?: string[] }).permissionRules!.join(', ')}]` + ? ` ${buildHumanReadableRuleLabel((infoProps as { permissionRules?: string[] }).permissionRules!)}` : ''; options.push({ - label: t('Always allow in this project') + rulesLabel, + label: friendlyLabel + ? t('Always allow {{action}} in this project', { + action: friendlyLabel.trim(), + }) + : t('Always allow in this project'), value: ToolConfirmationOutcome.ProceedAlwaysProject, key: 'Always allow in this project', }); options.push({ - label: t('Always allow for this user') + rulesLabel, + label: friendlyLabel + ? t('Always allow {{action}} for this user', { + action: friendlyLabel.trim(), + }) + : t('Always allow for this user'), value: ToolConfirmationOutcome.ProceedAlwaysUser, key: 'Always allow for this user', }); @@ -401,16 +421,24 @@ export const ToolConfirmationMessage: React.FC< key: 'Yes, allow once', }); if (isTrustedFolder && !confirmationDetails.hideAlwaysAllow) { - const rulesLabel = mcpProps.permissionRules?.length - ? ` [${mcpProps.permissionRules.join(', ')}]` + const friendlyLabel = mcpProps.permissionRules?.length + ? ` ${buildHumanReadableRuleLabel(mcpProps.permissionRules)}` : ''; options.push({ - label: t('Always allow in this project') + rulesLabel, + label: friendlyLabel + ? t('Always allow {{action}} in this project', { + action: friendlyLabel.trim(), + }) + : t('Always allow in this project'), value: ToolConfirmationOutcome.ProceedAlwaysProject, key: 'Always allow in this project', }); options.push({ - label: t('Always allow for this user') + rulesLabel, + label: friendlyLabel + ? t('Always allow {{action}} for this user', { + action: friendlyLabel.trim(), + }) + : t('Always allow for this user'), value: ToolConfirmationOutcome.ProceedAlwaysUser, key: 'Always allow for this user', }); diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 097120d08c..7279d452b8 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -701,7 +701,13 @@ export class CoreToolScheduler { // This check should happen before registry lookup to provide a clear permission error const pm = this.config.getPermissionManager?.(); if (pm && !pm.isToolEnabled(reqInfo.name)) { - const permissionErrorMessage = `Qwen Code requires permission to use "${reqInfo.name}", but that permission was declined.`; + const matchingRule = pm.findMatchingDenyRule({ + toolName: reqInfo.name, + }); + const ruleInfo = matchingRule + ? ` Matching deny rule: "${matchingRule}".` + : ''; + const permissionErrorMessage = `Qwen Code requires permission to use "${reqInfo.name}", but that permission was declined.${ruleInfo}`; return { status: 'error', request: reqInfo, @@ -914,10 +920,16 @@ export class CoreToolScheduler { if (finalPermission === 'deny') { // Hard deny: security violation or PM explicit deny - const denyMessage = - defaultPermission === 'deny' - ? `Tool "${reqInfo.name}" is denied: command substitution is not allowed for security reasons.` - : `Tool "${reqInfo.name}" is denied by permission rules.`; + let denyMessage: string; + if (defaultPermission === 'deny') { + denyMessage = `Tool "${reqInfo.name}" is denied: command substitution is not allowed for security reasons.`; + } else { + const matchingRule = pm?.findMatchingDenyRule(pmCtx); + const ruleInfo = matchingRule + ? ` Matching deny rule: "${matchingRule}".` + : ''; + denyMessage = `Tool "${reqInfo.name}" is denied by permission rules.${ruleInfo}`; + } this.setStatusInternal( reqInfo.callId, 'error', @@ -1002,7 +1014,7 @@ export class CoreToolScheduler { this.config.getInputFormat() !== InputFormat.STREAM_JSON; if (shouldAutoDeny) { - const errorMessage = `Qwen Code requires permission to use "${reqInfo.name}", but that permission was declined.`; + const errorMessage = `Qwen Code requires permission to use "${reqInfo.name}", but that permission was declined (non-interactive mode cannot prompt for confirmation).`; this.setStatusInternal( reqInfo.callId, 'error', diff --git a/packages/core/src/permissions/permission-manager.test.ts b/packages/core/src/permissions/permission-manager.test.ts index d15f36b258..94a5126baf 100644 --- a/packages/core/src/permissions/permission-manager.test.ts +++ b/packages/core/src/permissions/permission-manager.test.ts @@ -20,6 +20,7 @@ import { splitCompoundCommand, buildPermissionRules, getRuleDisplayName, + buildHumanReadableRuleLabel, } from './rule-parser.js'; import { PermissionManager } from './permission-manager.js'; import type { PermissionManagerConfig } from './permission-manager.js'; @@ -1519,3 +1520,174 @@ describe('buildPermissionRules', () => { }); }); }); + +// ─── buildHumanReadableRuleLabel ───────────────────────────────────────────── + +describe('buildHumanReadableRuleLabel', () => { + it('returns empty string for empty rules array', () => { + expect(buildHumanReadableRuleLabel([])).toBe(''); + }); + + it('converts bare Read rule to "read files"', () => { + expect(buildHumanReadableRuleLabel(['Read'])).toBe('read files'); + }); + + it('converts bare Bash rule to "run commands"', () => { + expect(buildHumanReadableRuleLabel(['Bash'])).toBe('run commands'); + }); + + it('converts bare WebSearch rule to "search the web"', () => { + expect(buildHumanReadableRuleLabel(['WebSearch'])).toBe('search the web'); + }); + + it('converts Read with absolute path specifier', () => { + const label = buildHumanReadableRuleLabel(['Read(//Users/mochi/.qwen/**)']); + expect(label).toBe('read files in /Users/mochi/.qwen/'); + }); + + it('converts Read with relative path specifier', () => { + const label = buildHumanReadableRuleLabel(['Read(/src/**)']); + expect(label).toBe('read files in /src/'); + }); + + it('converts Edit with path specifier', () => { + const label = buildHumanReadableRuleLabel(['Edit(//tmp/**)']); + expect(label).toBe('edit files in /tmp/'); + }); + + it('converts Bash with command specifier', () => { + const label = buildHumanReadableRuleLabel(['Bash(git *)']); + expect(label).toBe("run 'git *' commands"); + }); + + it('converts WebFetch with domain specifier', () => { + const label = buildHumanReadableRuleLabel(['WebFetch(github.com)']); + expect(label).toBe('fetch from github.com'); + }); + + it('converts Skill with literal specifier', () => { + const label = buildHumanReadableRuleLabel(['Skill(Explore)']); + expect(label).toBe('use skill "Explore"'); + }); + + it('converts Agent with literal specifier', () => { + const label = buildHumanReadableRuleLabel(['Agent(research)']); + expect(label).toBe('use agent "research"'); + }); + + it('joins multiple rules with commas', () => { + const label = buildHumanReadableRuleLabel([ + 'Read(//Users/alice/**)', + 'Bash(npm *)', + ]); + expect(label).toBe("read files in /Users/alice/, run 'npm *' commands"); + }); + + it('handles unknown display names gracefully', () => { + const label = buildHumanReadableRuleLabel(['mcp__server__tool']); + expect(label).toBe('mcp__server__tool'); + }); + + it('handles unknown display name with specifier', () => { + const label = buildHumanReadableRuleLabel(['UnknownCategory(someValue)']); + expect(label).toBe('unknowncategory "someValue"'); + }); + + it('cleans path with /* suffix', () => { + const label = buildHumanReadableRuleLabel(['Read(//home/user/docs/*)']); + expect(label).toBe('read files in /home/user/docs/'); + }); + + it('round-trips from buildPermissionRules for file tool', () => { + const rules = buildPermissionRules({ + toolName: 'read_file', + filePath: '/Users/alice/.secrets', + }); + const label = buildHumanReadableRuleLabel(rules); + expect(label).toBe('read files in /Users/alice/'); + }); + + it('round-trips from buildPermissionRules for shell command', () => { + const rules = buildPermissionRules({ + toolName: 'run_shell_command', + command: 'git status', + }); + const label = buildHumanReadableRuleLabel(rules); + expect(label).toBe("run 'git status' commands"); + }); + + it('round-trips from buildPermissionRules for web fetch', () => { + const rules = buildPermissionRules({ + toolName: 'web_fetch', + domain: 'example.com', + }); + const label = buildHumanReadableRuleLabel(rules); + expect(label).toBe('fetch from example.com'); + }); +}); + +// ─── PermissionManager.findMatchingDenyRule ────────────────────────────────── + +describe('PermissionManager.findMatchingDenyRule', () => { + it('returns the raw deny rule string when context matches', () => { + const pm = new PermissionManager( + makeConfig({ permissionsDeny: ['Bash(rm *)'] }), + ); + pm.initialize(); + + const result = pm.findMatchingDenyRule({ + toolName: 'run_shell_command', + command: 'rm -rf /tmp/foo', + }); + expect(result).toBe('Bash(rm *)'); + }); + + it('returns undefined when no deny rule matches', () => { + const pm = new PermissionManager( + makeConfig({ permissionsDeny: ['Bash(rm *)'] }), + ); + pm.initialize(); + + const result = pm.findMatchingDenyRule({ + toolName: 'run_shell_command', + command: 'git status', + }); + expect(result).toBeUndefined(); + }); + + it('matches session deny rules', () => { + const pm = new PermissionManager(makeConfig()); + pm.initialize(); + pm.addSessionDenyRule('Read(//secret/**)'); + + const result = pm.findMatchingDenyRule({ + toolName: 'read_file', + filePath: '/secret/key.pem', + }); + expect(result).toBe('Read(//secret/**)'); + }); + + it('returns undefined for non-denied tool', () => { + const pm = new PermissionManager( + makeConfig({ permissionsDeny: ['ShellTool'] }), + ); + pm.initialize(); + + const result = pm.findMatchingDenyRule({ toolName: 'read_file' }); + expect(result).toBeUndefined(); + }); + + it('matches bare tool deny rule', () => { + const pm = new PermissionManager( + makeConfig({ permissionsDeny: ['ShellTool'] }), + ); + pm.initialize(); + + const result = pm.findMatchingDenyRule({ + toolName: 'run_shell_command', + command: 'echo hello', + }); + // rule.raw preserves the original rule string as written in config + expect(result).toBe('ShellTool'); + }); +}); diff --git a/packages/core/src/permissions/permission-manager.ts b/packages/core/src/permissions/permission-manager.ts index 06f0548b03..f10d1d4a36 100644 --- a/packages/core/src/permissions/permission-manager.ts +++ b/packages/core/src/permissions/permission-manager.ts @@ -365,6 +365,43 @@ export class PermissionManager { return decision !== 'deny'; } + /** + * Find the first deny rule that matches the given context. + * Returns the raw rule string if found, or undefined if no deny rule matches. + * + * Useful for providing user-visible feedback about which rule caused a denial. + */ + findMatchingDenyRule(ctx: PermissionCheckContext): string | undefined { + const { toolName, command, filePath, domain, specifier } = ctx; + + const pathCtx: PathMatchContext | undefined = + this.config.getProjectRoot && this.config.getCwd + ? { + projectRoot: this.config.getProjectRoot(), + cwd: this.config.getCwd(), + } + : undefined; + + const matchArgs = [ + toolName, + command, + filePath, + domain, + pathCtx, + specifier, + ] as const; + + for (const rule of [ + ...this.sessionRules.deny, + ...this.persistentRules.deny, + ]) { + if (matchesRule(rule, ...matchArgs)) { + return rule.raw; + } + } + return undefined; + } + // --------------------------------------------------------------------------- // Shell command helper // --------------------------------------------------------------------------- diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index 32c413081f..6ca9e83632 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -405,6 +405,106 @@ export function buildPermissionRules(ctx: PermissionCheckContext): string[] { } } +/** + * Human-readable display names for permission rule categories. + * Maps display name → verb phrase for use in "Always allow [verb phrase] in this project". + */ +const DISPLAY_NAME_TO_VERB: Readonly> = { + Read: 'read files', + Edit: 'edit files', + Bash: 'run commands', + WebFetch: 'fetch from', + WebSearch: 'search the web', + Agent: 'use agent', + Skill: 'use skill', + SaveMemory: 'save memory', + TodoWrite: 'write todos', + Lsp: 'use LSP', + ExitPlanMode: 'exit plan mode', +}; + +/** + * Strip the glob suffix (e.g. `/**`) and the leading `//` from an absolute + * path specifier so it reads cleanly in a UI label. + * + * `//Users/mochi/.qwen/**` → `/Users/mochi/.qwen/` + * `/src/**` → `src/` + */ +function cleanPathSpecifier(specifier: string): string { + let cleaned = specifier; + // Remove trailing glob patterns like /** or /* + cleaned = cleaned.replace(/\/\*\*$/, '/').replace(/\/\*$/, '/'); + // Convert rule grammar `//absolute` → `/absolute` + if (cleaned.startsWith('//')) { + cleaned = cleaned.substring(1); + } + // Ensure trailing slash for directories + if (!cleaned.endsWith('/')) { + cleaned += '/'; + } + return cleaned; +} + +/** + * Build a human-readable label describing what a set of permission rules allow. + * + * Used in "Always Allow" UI options to give users a clear, natural-language + * description instead of raw rule syntax. + * + * Examples: + * `["Read(//Users/mochi/.qwen/**)"]` → `"read files in /Users/mochi/.qwen/"` + * `["Bash(git *)"]` → `"run 'git *' commands"` + * `["WebFetch(github.com)"]` → `"fetch from github.com"` + * `["Read"]` → `"read files"` + * + * @param rules - Array of rule strings from buildPermissionRules() + * @returns A human-readable description string + */ +export function buildHumanReadableRuleLabel(rules: string[]): string { + if (!rules.length) return ''; + + const parts: string[] = []; + for (const rule of rules) { + // Parse "DisplayName(specifier)" or bare "DisplayName" + const parenIdx = rule.indexOf('('); + if (parenIdx === -1) { + // Bare rule like "Read" or "Bash" + const verb = DISPLAY_NAME_TO_VERB[rule] ?? rule.toLowerCase(); + parts.push(verb); + continue; + } + + const displayName = rule.substring(0, parenIdx); + const specifier = rule.substring(parenIdx + 1, rule.length - 1); // strip parens + const verb = DISPLAY_NAME_TO_VERB[displayName] ?? displayName.toLowerCase(); + + const canonicalName = Object.entries(CANONICAL_TO_RULE_DISPLAY).find( + ([, v]) => v === displayName, + )?.[0]; + const kind = canonicalName ? getSpecifierKind(canonicalName) : 'literal'; + + switch (kind) { + case 'path': { + const cleanPath = cleanPathSpecifier(specifier); + parts.push(`${verb} in ${cleanPath}`); + break; + } + case 'command': + parts.push(`run '${specifier}' commands`); + break; + case 'domain': + parts.push(`${verb} ${specifier}`); + break; + case 'literal': + default: + parts.push(`${verb} "${specifier}"`); + break; + } + } + + return parts.join(', '); +} + // ───────────────────────────────────────────────────────────────────────────── // Shell command matching // ───────────────────────────────────────────────────────────────────────────── diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index dc1537930d..24be79d2ef 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -366,6 +366,87 @@ describe('GlobTool', () => { }); }); + describe('multi-directory workspace', () => { + it('should search across all workspace directories when no path is specified', async () => { + // Create a second workspace directory + const secondDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'glob-tool-second-'), + ); + await fs.writeFile(path.join(secondDir, '.git'), ''); // Fake git repo + await fs.writeFile(path.join(secondDir, 'extra.txt'), 'extra content'); + await fs.writeFile(path.join(secondDir, 'bonus.txt'), 'bonus content'); + + const multiDirConfig = { + ...mockConfig, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [secondDir]), + } as unknown as Config; + + const multiDirGlobTool = new GlobTool(multiDirConfig); + const params: GlobToolParams = { pattern: '*.txt' }; + const invocation = multiDirGlobTool.build(params); + const result = await invocation.execute(abortSignal); + + // Should find files from both directories + expect(result.llmContent).toContain(path.join(tempRootDir, 'fileA.txt')); + expect(result.llmContent).toContain(path.join(secondDir, 'extra.txt')); + expect(result.llmContent).toContain(path.join(secondDir, 'bonus.txt')); + expect(result.llmContent).toContain('across 2 workspace directories'); + + await fs.rm(secondDir, { recursive: true, force: true }); + }); + + it('should deduplicate entries across overlapping directories', async () => { + // Use the same directory twice to test deduplication + const multiDirConfig = { + ...mockConfig, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [tempRootDir]), + } as unknown as Config; + + const multiDirGlobTool = new GlobTool(multiDirConfig); + const params: GlobToolParams = { pattern: '*.txt' }; + const invocation = multiDirGlobTool.build(params); + const result = await invocation.execute(abortSignal); + + // Should still only have 2 txt files (fileA.txt, FileB.TXT), not doubled + expect(result.llmContent).toContain('Found 2 file(s)'); + }); + + it('should use single directory description when only one workspace dir', async () => { + const params: GlobToolParams = { pattern: '*.txt' }; + const invocation = globTool.build(params); + const result = await invocation.execute(abortSignal); + + expect(result.llmContent).toContain('in the workspace directory'); + expect(result.llmContent).not.toContain('across'); + }); + + it('should search only the specified path when path is provided (ignoring multi-dir)', async () => { + const secondDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'glob-tool-second-'), + ); + await fs.writeFile(path.join(secondDir, '.git'), ''); + await fs.writeFile(path.join(secondDir, 'other.txt'), 'other'); + + const multiDirConfig = { + ...mockConfig, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [secondDir]), + } as unknown as Config; + + const multiDirGlobTool = new GlobTool(multiDirConfig); + const params: GlobToolParams = { pattern: '*.txt', path: 'sub' }; + const invocation = multiDirGlobTool.build(params); + const result = await invocation.execute(abortSignal); + + // Should NOT find files from secondDir + expect(result.llmContent).not.toContain('other.txt'); + + await fs.rm(secondDir, { recursive: true, force: true }); + }); + }); + describe('ignore file handling', () => { it('should respect .gitignore files by default', async () => { await fs.writeFile(path.join(tempRootDir, '.gitignore'), '*.ignored.txt'); diff --git a/packages/core/src/tools/grep.test.ts b/packages/core/src/tools/grep.test.ts index 5a07dcadac..868cecd784 100644 --- a/packages/core/src/tools/grep.test.ts +++ b/packages/core/src/tools/grep.test.ts @@ -357,6 +357,48 @@ describe('GrepTool', () => { // Clean up await fs.rm(secondDir, { recursive: true, force: true }); }); + + it('should convert relative paths to absolute when searching multiple directories', async () => { + const secondDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'grep-tool-second-'), + ); + await fs.writeFile( + path.join(secondDir, 'extra.txt'), + 'world content in second dir', + ); + + const multiDirConfig = { + getTargetDir: () => tempRootDir, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [secondDir]), + getFileExclusions: () => ({ + getGlobExcludes: () => [], + }), + getTruncateToolOutputThreshold: () => 25000, + getTruncateToolOutputLines: () => 1000, + } as unknown as Config; + + const multiDirGrepTool = new GrepTool(multiDirConfig); + + const params: GrepToolParams = { pattern: 'world' }; + const invocation = multiDirGrepTool.build(params); + const result = await invocation.execute(abortSignal); + + // Should show "across N workspace directories" + expect(result.llmContent).toContain('across 2 workspace directories'); + + // File paths from the second directory should be absolute + expect(result.llmContent).toContain( + `File: ${path.resolve(secondDir, 'extra.txt')}`, + ); + + // File paths from the first directory should also be absolute + expect(result.llmContent).toContain( + `File: ${path.resolve(tempRootDir, 'fileA.txt')}`, + ); + + await fs.rm(secondDir, { recursive: true, force: true }); + }); }); describe('getDescription', () => { diff --git a/packages/core/src/tools/ripGrep.test.ts b/packages/core/src/tools/ripGrep.test.ts index 05730a7e9b..5edbc680a7 100644 --- a/packages/core/src/tools/ripGrep.test.ts +++ b/packages/core/src/tools/ripGrep.test.ts @@ -436,6 +436,116 @@ describe('RipGrepTool', () => { }); }); + describe('multi-directory workspace', () => { + it('should search across all workspace directories when no path is specified', async () => { + const secondDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'grep-tool-second-'), + ); + await fs.writeFile( + path.join(secondDir, 'extra.txt'), + 'hello from second dir', + ); + + const multiDirConfig = { + ...mockConfig, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [secondDir]), + } as unknown as Config; + + const multiDirGrepTool = new RipGrepTool(multiDirConfig); + + (runRipgrep as Mock).mockResolvedValue({ + stdout: `fileA.txt:1:hello world${EOL}${secondDir}/extra.txt:1:hello from second dir${EOL}`, + truncated: false, + error: undefined, + }); + + const params: RipGrepToolParams = { pattern: 'hello' }; + const invocation = multiDirGrepTool.build(params); + const result = await invocation.execute(abortSignal); + + expect(result.llmContent).toContain('across 2 workspace directories'); + expect(result.llmContent).toContain('Found 2 matches'); + + // Verify both paths were passed to runRipgrep + expect(runRipgrep).toHaveBeenCalledWith( + expect.arrayContaining([tempRootDir, secondDir]), + expect.anything(), + ); + + await fs.rm(secondDir, { recursive: true, force: true }); + }); + + it('should search only specified path when path is given (ignoring multi-dir)', async () => { + const secondDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'grep-tool-second-'), + ); + await fs.writeFile(path.join(secondDir, 'other.txt'), 'other content'); + + const multiDirConfig = { + ...mockConfig, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [secondDir]), + } as unknown as Config; + + const multiDirGrepTool = new RipGrepTool(multiDirConfig); + + (runRipgrep as Mock).mockResolvedValue({ + stdout: `fileC.txt:1:another world in sub dir${EOL}`, + truncated: false, + error: undefined, + }); + + const params: RipGrepToolParams = { pattern: 'world', path: 'sub' }; + const invocation = multiDirGrepTool.build(params); + const result = await invocation.execute(abortSignal); + + expect(result.llmContent).toContain('in path "sub"'); + expect(result.llmContent).not.toContain('across'); + + await fs.rm(secondDir, { recursive: true, force: true }); + }); + + it('should load .qwenignore from each workspace directory', async () => { + const secondDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'grep-tool-second-'), + ); + await fs.writeFile(path.join(secondDir, '.qwenignore'), 'ignored.txt\n'); + await fs.writeFile( + path.join(tempRootDir, '.qwenignore'), + 'other-ignored.txt\n', + ); + + const multiDirConfig = { + ...mockConfig, + getWorkspaceContext: () => + createMockWorkspaceContext(tempRootDir, [secondDir]), + } as unknown as Config; + + const multiDirGrepTool = new RipGrepTool(multiDirConfig); + + (runRipgrep as Mock).mockResolvedValue({ + stdout: '', + truncated: false, + error: undefined, + }); + + const params: RipGrepToolParams = { pattern: 'test' }; + const invocation = multiDirGrepTool.build(params); + await invocation.execute(abortSignal); + + // Verify both .qwenignore files were passed + const rgArgs = (runRipgrep as Mock).mock.calls[0][0] as string[]; + const ignoreFileArgs = rgArgs.filter( + (a: string, i: number) => i > 0 && rgArgs[i - 1] === '--ignore-file', + ); + expect(ignoreFileArgs).toContain(path.join(tempRootDir, '.qwenignore')); + expect(ignoreFileArgs).toContain(path.join(secondDir, '.qwenignore')); + + await fs.rm(secondDir, { recursive: true, force: true }); + }); + }); + describe('abort signal handling', () => { it('should handle AbortSignal during search', async () => { const controller = new AbortController();