-
Notifications
You must be signed in to change notification settings - Fork 4.7k
fix: exclude tools field from agent config spread #7237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: exclude tools field from agent config spread #7237
Conversation
Fixes subagent permission bug where tools field was spread into resolved agent config, causing permission blocks to be ignored. Root cause: config.ts line 529 spread ...agent which included tools. Fix: destructure to exclude tools before spreading. Relates to: anomalyco#6527
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
|
After further investigation, we found a SECOND issue that also needs to be addressed: Issue 1 (this PR): The We're working on a second fix for this and will either:
Will update shortly. |
Adds permission-based filtering to ToolRegistry.tools() so subagents only see tools they're allowed to use based on their permission config. - Check agent.permission if provided - Use PermissionNext.evaluate() to check each tool - Filter out tools with 'deny' action This complements the config.ts fix that excludes the tools field from agent config spread. Fixes anomalyco#6527
|
✅ Second fix added Just pushed the complete fix. This PR now includes:
The
Together, these fixes ensure subagents only see tools they're permitted to use. |
The previous fix incorrectly filtered out tools like bash when they had
permission rules with a default deny (e.g., { 'git*': 'allow', '*': 'deny' }).
New logic:
- If permission is an object with rules, allow the tool (rules enforced at execution)
- If permission is explicitly 'deny' string, filter out the tool
- If permission is 'allow', 'ask', or undefined, allow the tool
This ensures git-agent gets bash tool access while still respecting
command-level restrictions at execution time.
Previous fix used PermissionNext.evaluate(toolId, '*', permission) which incorrectly matched catch-all deny rules. Correct logic: - If permission[toolId] is an object (has rules), allow the tool - If permission[toolId] is 'deny' string, filter out the tool - Command-level restrictions enforced at execution time
Final Fix Applied (v1.0.220-fix4)The complete fix is now in this PR. Key insight: The Problem with Previous ApproachUsing
Correct ApproachCheck the permission TYPE directly: if (agent?.permission) {
const toolPermission = (agent.permission as unknown as Record<string, unknown>)[t.id]
// Object with rules → allow tool (command restrictions at execution time)
if (toolPermission !== null && typeof toolPermission === 'object') {
return true
}
// Explicit 'deny' string → filter out tool
if (toolPermission === 'deny') {
return false
}
}Why This Works
Built and tested - git-agent now properly receives bash tool with command restrictions. |
- Check permission array for tool-specific rules - Only filter out tools where ALL rules are deny - Support catch-all deny for tools without explicit rules - Add comprehensive test suite (12 tests) Fixes anomalyco#6527
|
|
Closing this PR as our fix doesn't address the actual root cause. What we found:
Our changes to Our plan:
|
Summary
Fixes subagent permission bug where tools field was spread into resolved agent config, causing permission blocks to be ignored for tool availability.
Root Cause
In packages/opencode/src/config/config.ts around line 529, the Agent schema transform spreads the entire agent object:
This preserves the tools field alongside the new permission field, causing tools to override permission-based tool availability.
The Fix
Destructure to exclude tools before spreading:
Testing
opencode debug agent <name>:Fixes #6527
Related to #6873