feat(core): honor skill allowedTools by auto-approving declared tools#4704
Open
tanzhenxin wants to merge 1 commit into
Open
feat(core): honor skill allowedTools by auto-approving declared tools#4704tanzhenxin wants to merge 1 commit into
tanzhenxin wants to merge 1 commit into
Conversation
Skills accepted an `allowedTools` frontmatter field, but it was parsed and then ignored (the type comment said "informational only (no gating)"), so every tool a skill drove still triggered a permission prompt. On skill invocation, each `allowedTools` entry is now added as a session-scoped permission allow rule, auto-approving matching tool calls for the rest of the session. This is an additive grant — it never restricts the tools the model can see. Applied at all three skill invocation paths (the model Skill tool, the user slash command for user/project/extension skills, and bundled-skill slash commands) via a shared `applySkillAllowedTools` helper.
Collaborator
Author
E2E Test ReportInteractive mode (tmux),
Group A confirms the declared tool is auto-approved only after the change. Group B (no declaration) still prompts in both builds, proving the suppression is attributable to |
Contributor
📋 Review SummaryThis PR implements the 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Skills accept an
allowedToolsfield in their frontmatter, but until now that field did nothing — it was read and then ignored. This PR makes it take effect: when a skill runs, the tools it declares are auto-approved for the rest of the session, so the skill's own tool calls no longer interrupt the user with permission prompts.It is an additive grant — the declared tools are pre-approved, and nothing is ever hidden or removed from what the model can use. The grant applies however a skill is launched: when the model picks it, and when a user types its slash command (including the built-in skills such as
/review).Why it's needed
A skill author can list the tools their skill needs, but that list had no effect — every command the skill drove still popped a permission prompt, which defeats the purpose of declaring the tools up front. Honoring
allowedToolsmakes those declarations meaningful and gives a skill a smooth, prompt-free run for exactly the tools it asks for.Reviewer Test Plan
How to verify
Create a project skill at
.qwen/skills/grant-touch/SKILL.md:Start the CLI with
qwen --approval-mode default, then type/grant-touch.Allow execution of: 'touch'?— and the command is blocked until you approve. The declaredallowedToolshad no effect./tmp/qwen-allowtools-okis created.Control check: the same skill without an
allowedToolsblock still shows the permission dialog — confirming the auto-approval comes specifically from the declared tools, not from the command being inherently allowed.Evidence (Before & After)
/grant-touch(declaresBash(touch *))allowedTools)Tested on
Environment (optional)
Local build, interactive mode via tmux,
--approval-mode default. Plus unit tests covering the shared grant helper and all three skill invocation paths.Risk & Scope
Bash) would auto-approve broadly. Authors should scope entries narrowly (e.g.Bash(git *)). The grant is session-scoped, and AUTO mode still withholds dangerous broad grants from the classifier.Linked Issues
N/A
中文说明
这个 PR 做了什么
Skill 的 frontmatter 支持
allowedTools字段，但此前该字段不起任何作用——它被解析后就被忽略了。本 PR 让它真正生效：当一个 skill 运行时，它声明的工具会在本次会话中被自动批准，因此 skill 自身发起的工具调用不再用权限确认弹窗打断用户。这是一种“附加授权”——被声明的工具会被预先批准，而模型可用的工具集不会被隐藏或删减。无论 skill 以何种方式启动都会应用该授权：模型主动选择它时，以及用户输入其斜杠命令时（包括
/review等内置 skill）。为什么需要
Skill 作者可以列出 skill 所需的工具，但这个列表此前毫无效果——skill 驱动的每个命令仍会弹出权限确认，这违背了预先声明工具的初衷。让
allowedTools生效使这些声明变得有意义，并让 skill 针对其所声明的工具获得顺畅、无弹窗的运行体验。复核测试方案
如何验证
在
.qwen/skills/grant-touch/SKILL.md创建一个项目级 skill：使用
qwen --approval-mode default启动 CLI，然后输入/grant-touch。Allow execution of: 'touch'?——命令在你批准前被阻塞。声明的allowedTools不起作用。/tmp/qwen-allowtools-ok被创建。对照验证：同一个 skill 在没有
allowedTools块时仍会显示权限弹窗——这证明自动批准确实来自所声明的工具，而非该命令本身被默认允许。证据（前后对比）
/grant-touch（声明了Bash(touch *)）allowedTools）测试平台
仅在 macOS 上本地验证（交互模式 +
--approval-mode default），并补充了覆盖共享授权辅助函数与全部三条 skill 调用路径的单元测试。风险与范围
Bash）的 skill 会带来宽泛的自动批准。作者应将条目限定到较窄范围（如Bash(git *)）。该授权是会话级的，且 AUTO 模式仍会把危险的宽泛授权从分类器中暂存隔离。