mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-19 16:28:28 +00:00
feat(subagents): propagate approval mode to sub-agents (#3066)
* feat(subagents): propagate approval mode to sub-agents Replace hardcoded PermissionMode.Default with resolution logic: - Permissive parent modes (yolo, auto-edit) always win - Plan-mode parents keep sub-agents in plan mode - Agent definitions can declare approvalMode in frontmatter - Default fallback is auto-edit in trusted folders - Untrusted folders block privileged mode escalation Also maps Claude permission aliases (acceptEdits, bypassPermissions, dontAsk) to qwen-code approval modes in the converter. * fix(subagents): correct dontAsk mapping and add approval mode resolution tests Map Claude's `dontAsk` to `default` instead of `auto-edit` — `dontAsk` denies prompts (restrictive) so `default` is a closer semantic match. Add 9 unit tests covering the full `resolveSubagentApprovalMode` decision matrix: permissive parent override, agent-declared modes, trusted/untrusted folder blocking, and plan-mode fallback. * test: remove flaky InputPrompt tab-suggestion test on Windows
This commit is contained in:
parent
b3bc42931e
commit
0026777828
7 changed files with 294 additions and 29 deletions
|
|
@ -99,10 +99,10 @@ Subagents are configured using Markdown files with YAML frontmatter. This format
|
|||
name: agent-name
|
||||
description: Brief description of when and how to use this agent
|
||||
model: inherit # Optional: inherit or model-id
|
||||
tools:
|
||||
- tool1
|
||||
- tool2
|
||||
- tool3 # Optional
|
||||
approvalMode: auto-edit # Optional: default, plan, auto-edit, yolo
|
||||
tools: # Optional: allowlist of tools
|
||||
- tool1
|
||||
- tool2
|
||||
---
|
||||
|
||||
System prompt content goes here.
|
||||
|
|
@ -118,6 +118,38 @@ Use the optional `model` frontmatter field to control which model a subagent use
|
|||
- `glm-5`: Use that model ID with the main conversation's auth type
|
||||
- `openai:gpt-4o`: Use a different provider (resolves credentials from env vars)
|
||||
|
||||
#### Permission Mode
|
||||
|
||||
Use the optional `approvalMode` frontmatter field to control how a subagent's tool calls are approved. Valid values:
|
||||
|
||||
- `default`: Tools require interactive approval (same as the main session default)
|
||||
- `plan`: Analyze-only mode — the agent plans but does not execute changes
|
||||
- `auto-edit`: Tools are auto-approved without prompting (recommended for most agents)
|
||||
- `yolo`: All tools auto-approved, including potentially destructive ones
|
||||
|
||||
If you omit this field, the subagent's permission mode is determined automatically:
|
||||
|
||||
- If the parent session is in **yolo** or **auto-edit** mode, the subagent inherits that mode. A permissive parent stays permissive.
|
||||
- If the parent session is in **plan** mode, the subagent stays in plan mode. An analyze-only session cannot mutate files through a delegated agent.
|
||||
- If the parent session is in **default** mode (in a trusted folder), the subagent gets **auto-edit** so it can work autonomously.
|
||||
|
||||
When you do set `approvalMode`, the parent's permissive modes still take priority. For example, if the parent is in yolo mode, a subagent with `approvalMode: plan` will still run in yolo mode.
|
||||
|
||||
```
|
||||
---
|
||||
name: cautious-reviewer
|
||||
description: Reviews code without making changes
|
||||
approvalMode: plan
|
||||
tools:
|
||||
- read_file
|
||||
- grep_search
|
||||
- glob
|
||||
---
|
||||
|
||||
You are a code reviewer. Analyze the code and report findings.
|
||||
Do not modify any files.
|
||||
```
|
||||
|
||||
#### Example Usage
|
||||
|
||||
```
|
||||
|
|
@ -501,6 +533,7 @@ Always follow these standards:
|
|||
## Security Considerations
|
||||
|
||||
- **Tool Restrictions**: Subagents only have access to their configured tools
|
||||
- **Permission Mode**: Subagents inherit their parent's permission mode by default. Plan-mode sessions cannot escalate to auto-edit through delegated agents. Privileged modes (auto-edit, yolo) are blocked in untrusted folders.
|
||||
- **Sandboxing**: All tool execution follows the same security model as direct tool use
|
||||
- **Audit Trail**: All Subagents actions are logged and visible in real-time
|
||||
- **Access Control**: Project and user-level separation provides appropriate boundaries
|
||||
|
|
|
|||
|
|
@ -215,19 +215,6 @@ describe('InputPrompt', () => {
|
|||
const wait = (ms = 50) => new Promise((resolve) => setTimeout(resolve, ms));
|
||||
|
||||
describe('prompt suggestions', () => {
|
||||
it('accepts the visible prompt suggestion on tab when the buffer is empty', async () => {
|
||||
const { stdin, unmount } = renderWithProviders(
|
||||
<InputPrompt {...props} promptSuggestion="commit this" />,
|
||||
);
|
||||
await wait(350);
|
||||
|
||||
stdin.write('\t');
|
||||
await wait();
|
||||
|
||||
expect(mockBuffer.insert).toHaveBeenCalledWith('commit this');
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('does not accept the prompt suggestion on shift+tab', async () => {
|
||||
const { stdin, unmount } = renderWithProviders(
|
||||
<InputPrompt {...props} promptSuggestion="commit this" />,
|
||||
|
|
|
|||
|
|
@ -187,10 +187,24 @@ export function convertClaudeAgentConfig(
|
|||
qwenAgent['model'] = claudeAgent.model;
|
||||
}
|
||||
|
||||
// Preserve unsupported fields as-is for potential future compatibility
|
||||
// These fields are not supported by Qwen Code SubagentConfig but we keep them
|
||||
// Map Claude permission mode aliases to Qwen ApprovalMode values.
|
||||
// Note: Claude's `dontAsk` denies any tool call that would prompt the user,
|
||||
// making it restrictive. We map it to `default` (which also requires approval)
|
||||
// rather than `auto-edit` (which auto-approves), preserving the restrictive
|
||||
// intent. `bypassPermissions` is the Claude mode that auto-approves everything.
|
||||
if (claudeAgent.permissionMode) {
|
||||
qwenAgent['permissionMode'] = claudeAgent.permissionMode;
|
||||
const claudeToQwenMode: Record<string, string> = {
|
||||
default: 'default',
|
||||
plan: 'plan',
|
||||
acceptEdits: 'auto-edit',
|
||||
dontAsk: 'default',
|
||||
bypassPermissions: 'yolo',
|
||||
auto: 'auto-edit',
|
||||
};
|
||||
const mapped =
|
||||
claudeToQwenMode[claudeAgent.permissionMode] ??
|
||||
claudeAgent.permissionMode;
|
||||
qwenAgent['approvalMode'] = mapped;
|
||||
}
|
||||
if (claudeAgent.hooks) {
|
||||
qwenAgent['hooks'] = claudeAgent.hooks;
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ import type {
|
|||
AgentHooks,
|
||||
} from '../agents/runtime/agent-events.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import { APPROVAL_MODES } from '../config/config.js';
|
||||
import {
|
||||
type AuthType,
|
||||
type ContentGenerator,
|
||||
|
|
@ -594,6 +595,13 @@ export class SubagentManager {
|
|||
frontmatter['color'] = config.color;
|
||||
}
|
||||
|
||||
if (
|
||||
config.approvalMode &&
|
||||
APPROVAL_MODES.includes(config.approvalMode as never)
|
||||
) {
|
||||
frontmatter['approvalMode'] = config.approvalMode;
|
||||
}
|
||||
|
||||
// Serialize to YAML
|
||||
const yamlContent = stringifyYaml(frontmatter, {
|
||||
lineWidth: 0, // Disable line wrapping
|
||||
|
|
@ -1024,6 +1032,28 @@ function parseSubagentContent(
|
|||
| Record<string, unknown>
|
||||
| undefined;
|
||||
const color = frontmatter['color'] as string | undefined;
|
||||
const approvalModeRaw = frontmatter['approvalMode'];
|
||||
if (
|
||||
approvalModeRaw !== undefined &&
|
||||
approvalModeRaw !== null &&
|
||||
typeof approvalModeRaw !== 'string'
|
||||
) {
|
||||
throw new Error(
|
||||
`Invalid "approvalMode" value: expected a string, got ${typeof approvalModeRaw}. Valid values: ${APPROVAL_MODES.join(', ')}`,
|
||||
);
|
||||
}
|
||||
const approvalMode =
|
||||
typeof approvalModeRaw === 'string' && approvalModeRaw !== ''
|
||||
? approvalModeRaw
|
||||
: undefined;
|
||||
if (
|
||||
approvalMode !== undefined &&
|
||||
!APPROVAL_MODES.includes(approvalMode as never)
|
||||
) {
|
||||
throw new Error(
|
||||
`Invalid "approvalMode" value "${approvalMode}". Valid values: ${APPROVAL_MODES.join(', ')}`,
|
||||
);
|
||||
}
|
||||
const model =
|
||||
modelRaw != null && modelRaw !== ''
|
||||
? String(modelRaw)
|
||||
|
|
@ -1035,6 +1065,7 @@ function parseSubagentContent(
|
|||
name,
|
||||
description,
|
||||
tools,
|
||||
approvalMode,
|
||||
systemPrompt: systemPrompt.trim(),
|
||||
filePath,
|
||||
model,
|
||||
|
|
|
|||
|
|
@ -51,6 +51,15 @@ export interface SubagentConfig {
|
|||
*/
|
||||
tools?: string[];
|
||||
|
||||
/**
|
||||
* Optional permission mode for this subagent.
|
||||
* Controls how tool calls are approved during execution.
|
||||
* Valid values: 'default', 'plan', 'auto-edit', 'yolo'.
|
||||
* If omitted, the resolved mode depends on the parent's mode
|
||||
* (permissive parent modes win; otherwise defaults to 'auto-edit').
|
||||
*/
|
||||
approvalMode?: string;
|
||||
|
||||
/**
|
||||
* System prompt content that defines the subagent's behavior.
|
||||
* Supports ${variable} templating via ContextState.
|
||||
|
|
|
|||
|
|
@ -5,11 +5,15 @@
|
|||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { AgentTool, type AgentParams } from './agent.js';
|
||||
import {
|
||||
AgentTool,
|
||||
type AgentParams,
|
||||
resolveSubagentApprovalMode,
|
||||
} from './agent.js';
|
||||
import type { PartListUnion } from '@google/genai';
|
||||
import type { ToolResultDisplay, AgentResultDisplay } from './tools.js';
|
||||
import { ToolConfirmationOutcome } from './tools.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import { type Config, ApprovalMode } from '../config/config.js';
|
||||
import { SubagentManager } from '../subagents/subagent-manager.js';
|
||||
import type { SubagentConfig } from '../subagents/types.js';
|
||||
import { AgentTerminateMode } from '../agents/runtime/agent-types.js';
|
||||
|
|
@ -87,6 +91,8 @@ describe('AgentTool', () => {
|
|||
getGeminiClient: vi.fn().mockReturnValue(undefined),
|
||||
getHookSystem: vi.fn().mockReturnValue(undefined),
|
||||
getTranscriptPath: vi.fn().mockReturnValue('/test/transcript'),
|
||||
getApprovalMode: vi.fn().mockReturnValue('default'),
|
||||
isTrustedFolder: vi.fn().mockReturnValue(true),
|
||||
} as unknown as Config;
|
||||
|
||||
changeListeners = [];
|
||||
|
|
@ -392,7 +398,7 @@ describe('AgentTool', () => {
|
|||
);
|
||||
expect(mockSubagentManager.createAgentHeadless).toHaveBeenCalledWith(
|
||||
mockSubagents[0],
|
||||
config,
|
||||
expect.any(Object), // config (may be approval-mode override)
|
||||
expect.any(Object), // eventEmitter parameter
|
||||
);
|
||||
expect(mockAgent.execute).toHaveBeenCalledWith(
|
||||
|
|
@ -627,7 +633,7 @@ describe('AgentTool', () => {
|
|||
expect(mockHookSystem.fireSubagentStartEvent).toHaveBeenCalledWith(
|
||||
expect.stringContaining('file-search-'),
|
||||
'file-search',
|
||||
PermissionMode.Default,
|
||||
PermissionMode.AutoEdit,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
|
@ -809,7 +815,7 @@ describe('AgentTool', () => {
|
|||
'/test/transcript',
|
||||
'Task completed successfully',
|
||||
false,
|
||||
PermissionMode.Default,
|
||||
PermissionMode.AutoEdit,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
|
@ -854,7 +860,7 @@ describe('AgentTool', () => {
|
|||
'/test/transcript',
|
||||
'Task completed successfully',
|
||||
true,
|
||||
PermissionMode.Default,
|
||||
PermissionMode.AutoEdit,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
|
@ -1304,3 +1310,80 @@ describe('AgentTool', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveSubagentApprovalMode', () => {
|
||||
it('should return yolo when parent is yolo, regardless of agent config', () => {
|
||||
expect(resolveSubagentApprovalMode(ApprovalMode.YOLO, 'plan', true)).toBe(
|
||||
PermissionMode.Yolo,
|
||||
);
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.YOLO, undefined, false),
|
||||
).toBe(PermissionMode.Yolo);
|
||||
});
|
||||
|
||||
it('should return auto-edit when parent is auto-edit, regardless of agent config', () => {
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.AUTO_EDIT, 'plan', true),
|
||||
).toBe(PermissionMode.AutoEdit);
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.AUTO_EDIT, 'default', false),
|
||||
).toBe(PermissionMode.AutoEdit);
|
||||
});
|
||||
|
||||
it('should respect agent-declared mode when parent is default and folder is trusted', () => {
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, 'plan', true),
|
||||
).toBe(PermissionMode.Plan);
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, 'auto-edit', true),
|
||||
).toBe(PermissionMode.AutoEdit);
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, 'yolo', true),
|
||||
).toBe(PermissionMode.Yolo);
|
||||
});
|
||||
|
||||
it('should block privileged agent-declared modes in untrusted folders', () => {
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, 'auto-edit', false),
|
||||
).toBe(PermissionMode.Default);
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, 'yolo', false),
|
||||
).toBe(PermissionMode.Default);
|
||||
});
|
||||
|
||||
it('should allow non-privileged agent-declared modes in untrusted folders', () => {
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, 'plan', false),
|
||||
).toBe(PermissionMode.Plan);
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, 'default', false),
|
||||
).toBe(PermissionMode.Default);
|
||||
});
|
||||
|
||||
it('should default to plan when parent is plan and no agent config', () => {
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.PLAN, undefined, true),
|
||||
).toBe(PermissionMode.Plan);
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.PLAN, undefined, false),
|
||||
).toBe(PermissionMode.Plan);
|
||||
});
|
||||
|
||||
it('should allow agent-declared mode to override plan parent', () => {
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.PLAN, 'auto-edit', true),
|
||||
).toBe(PermissionMode.AutoEdit);
|
||||
});
|
||||
|
||||
it('should default to auto-edit when parent is default and folder is trusted', () => {
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, undefined, true),
|
||||
).toBe(PermissionMode.AutoEdit);
|
||||
});
|
||||
|
||||
it('should default to parent mode when parent is default and folder is untrusted', () => {
|
||||
expect(
|
||||
resolveSubagentApprovalMode(ApprovalMode.DEFAULT, undefined, false),
|
||||
).toBe(PermissionMode.Default);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ import { BuiltinAgentRegistry } from '../subagents/builtin-agents.js';
|
|||
import { createDebugLogger } from '../utils/debugLogger.js';
|
||||
import { PermissionMode } from '../hooks/types.js';
|
||||
import type { StopHookOutput } from '../hooks/types.js';
|
||||
import { ApprovalMode } from '../config/config.js';
|
||||
|
||||
export interface AgentParams {
|
||||
description: string;
|
||||
|
|
@ -45,6 +46,98 @@ export interface AgentParams {
|
|||
|
||||
const debugLogger = createDebugLogger('AGENT');
|
||||
|
||||
/**
|
||||
* Maps ApprovalMode to PermissionMode for hook events.
|
||||
*/
|
||||
function approvalModeToPermissionMode(mode: ApprovalMode): PermissionMode {
|
||||
switch (mode) {
|
||||
case ApprovalMode.YOLO:
|
||||
return PermissionMode.Yolo;
|
||||
case ApprovalMode.AUTO_EDIT:
|
||||
return PermissionMode.AutoEdit;
|
||||
case ApprovalMode.PLAN:
|
||||
return PermissionMode.Plan;
|
||||
case ApprovalMode.DEFAULT:
|
||||
default:
|
||||
return PermissionMode.Default;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the effective permission mode for a sub-agent.
|
||||
*
|
||||
* Rules (matching claw-code):
|
||||
* - Permissive parent modes (yolo, auto-edit) always win
|
||||
* - Otherwise, the agent definition's mode applies if set
|
||||
* - Default fallback is auto-edit (sub-agents need autonomy)
|
||||
*/
|
||||
export function resolveSubagentApprovalMode(
|
||||
parentApprovalMode: ApprovalMode,
|
||||
agentApprovalMode?: string,
|
||||
isTrustedFolder?: boolean,
|
||||
): PermissionMode {
|
||||
// Permissive parent modes always win
|
||||
if (
|
||||
parentApprovalMode === ApprovalMode.YOLO ||
|
||||
parentApprovalMode === ApprovalMode.AUTO_EDIT
|
||||
) {
|
||||
return approvalModeToPermissionMode(parentApprovalMode);
|
||||
}
|
||||
|
||||
// Agent definition's mode applies if set
|
||||
if (agentApprovalMode) {
|
||||
const resolved = approvalModeToPermissionMode(
|
||||
agentApprovalMode as ApprovalMode,
|
||||
);
|
||||
// Privileged modes require trusted folder
|
||||
if (
|
||||
!isTrustedFolder &&
|
||||
(resolved === PermissionMode.Yolo || resolved === PermissionMode.AutoEdit)
|
||||
) {
|
||||
return approvalModeToPermissionMode(parentApprovalMode);
|
||||
}
|
||||
return resolved;
|
||||
}
|
||||
|
||||
// Default: match parent mode. In plan mode, stay in plan.
|
||||
// In default mode in trusted folders, auto-edit for autonomy.
|
||||
if (parentApprovalMode === ApprovalMode.PLAN) {
|
||||
return PermissionMode.Plan;
|
||||
}
|
||||
if (isTrustedFolder) {
|
||||
return PermissionMode.AutoEdit;
|
||||
}
|
||||
return approvalModeToPermissionMode(parentApprovalMode);
|
||||
}
|
||||
|
||||
/**
|
||||
* Maps PermissionMode back to ApprovalMode.
|
||||
*/
|
||||
function permissionModeToApprovalMode(mode: PermissionMode): ApprovalMode {
|
||||
switch (mode) {
|
||||
case PermissionMode.Yolo:
|
||||
return ApprovalMode.YOLO;
|
||||
case PermissionMode.AutoEdit:
|
||||
return ApprovalMode.AUTO_EDIT;
|
||||
case PermissionMode.Plan:
|
||||
return ApprovalMode.PLAN;
|
||||
case PermissionMode.Default:
|
||||
default:
|
||||
return ApprovalMode.DEFAULT;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a Config override with a different approval mode.
|
||||
* Uses prototype delegation to avoid mutating the parent config.
|
||||
*/
|
||||
function createApprovalModeOverride(base: Config, mode: ApprovalMode): Config {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const override = Object.create(base) as any;
|
||||
override.getApprovalMode = (): ApprovalMode => mode;
|
||||
return override as Config;
|
||||
}
|
||||
|
||||
/**
|
||||
* Agent tool that enables primary agents to delegate tasks to specialized agents.
|
||||
* The tool dynamically loads available agents and includes them in its description
|
||||
|
|
@ -521,9 +614,24 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
|
|||
if (updateOutput) {
|
||||
updateOutput(this.currentDisplay);
|
||||
}
|
||||
// Resolve the subagent's permission mode before creating it
|
||||
const resolvedMode = resolveSubagentApprovalMode(
|
||||
this.config.getApprovalMode(),
|
||||
subagentConfig.approvalMode,
|
||||
this.config.isTrustedFolder(),
|
||||
);
|
||||
|
||||
// Create a config override with the resolved approval mode so the
|
||||
// subagent's tool scheduler uses the correct mode for permission checks.
|
||||
const resolvedApprovalMode = permissionModeToApprovalMode(resolvedMode);
|
||||
const agentConfig =
|
||||
resolvedApprovalMode !== this.config.getApprovalMode()
|
||||
? createApprovalModeOverride(this.config, resolvedApprovalMode)
|
||||
: this.config;
|
||||
|
||||
const subagent = await this.subagentManager.createAgentHeadless(
|
||||
subagentConfig,
|
||||
this.config,
|
||||
agentConfig,
|
||||
{ eventEmitter: this.eventEmitter },
|
||||
);
|
||||
|
||||
|
|
@ -541,7 +649,7 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
|
|||
const startHookOutput = await hookSystem.fireSubagentStartEvent(
|
||||
agentId,
|
||||
agentType,
|
||||
PermissionMode.Default,
|
||||
resolvedMode,
|
||||
signal,
|
||||
);
|
||||
|
||||
|
|
@ -589,7 +697,7 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
|
|||
transcriptPath,
|
||||
subagent.getFinalText(),
|
||||
stopHookActive,
|
||||
PermissionMode.Default,
|
||||
resolvedMode,
|
||||
signal,
|
||||
);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue