Merge branch 'fix-permission-issues' into fix/acp-permission-flow

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';
@@ -1592,3 +1593,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');
+  });
+});

packages/core/src/permissions/permission-manager.ts

@@ -422,6 +422,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
   // ---------------------------------------------------------------------------

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<Record<string, string>> = {
+  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
 // ─────────────────────────────────────────────────────────────────────────────