-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Permission rework #6319
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
Permission rework #6319
Conversation
2964abd to
591cbd4
Compare
|
@copilot review this bitch |
|
/review |
| return null | ||
| } | ||
|
|
||
| function DialogPermission() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: This DialogPermission component is defined but never used or exported. Consider either completing and using it, or removing it to keep the codebase clean.
|
|
||
| export function Permission() { | ||
| const dialog = useDialog() | ||
| onMount(() => {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The Permission component has an empty onMount callback that does nothing. This could be removed unless there are plans to add functionality later.
| import { useTheme } from "../context/theme" | ||
|
|
||
| export function Permission() { | ||
| const dialog = useDialog() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The dialog variable is declared but never used in this component. Consider removing it or using it.
| @@ -0,0 +1,53 @@ | |||
| import { onMount } from "solid-js" | |||
| import { useDialog } from "../ui/dialog" | |||
| import { TextAttributes } from "@opentui/core" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The TextAttributes import is unused in this file (only used in the unused DialogPermission component). Consider removing it along with the cleanup of unused code.
| gap={1} | ||
| paddingLeft={2} | ||
| paddingRight={2} | ||
| onKeyDown={(e) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: There is a console.log statement here that appears to be debug code. Consider removing it before merging.
96b1106 to
6c21fd1
Compare
Replaces legacy Permission.respond with PermissionNext.reply for better async handling and updates server endpoints to use the new permission system. Improves error handling in session processor to work with both old and new permission rejection types.
Removes permission setting from opencode config and adds test root configuration to prevent running tests from project root.
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
## Problem The 'Hide tool details' toggle in the command palette (Ctrl+P) has no effect on the UI - completed tools are always displayed regardless of the setting. ## Root Cause The Permission rework commit 351ddee (merged 2026-01-01, PR anomalyco#6319) accidentally removed the showDetails check from the ToolPart function when refactoring the tool rendering system. ### Before Permission rework (working): ```typescript function ToolPart(props: { last: boolean; part: ToolPart; message: AssistantMessage }) { const { theme } = useTheme() const { showDetails } = use() // <-- Gets showDetails from context const sync = useSync() const [margin, setMargin] = createSignal(0) const component = createMemo(() => { // Hide tool if showDetails is false and tool completed successfully // But always show if there's an error or permission is required const shouldHide = !showDetails() && props.part.state.status === "completed" && !sync.data.permission[props.message.sessionID]?.some((x) => x.callID === props.part.callID) if (shouldHide) { return undefined // <-- Returns nothing, hiding the tool } // ... }) } ``` ### After Permission rework (broken): ```typescript function ToolPart(props: { last: boolean; part: ToolPart; message: AssistantMessage }) { const sync = useSync() // <-- No use() call! // <-- No showDetails check! const toolprops = { /* ... */ } return ( <Switch> // <-- Always renders all tools unconditionally </Switch> ) } ``` ## Fix Restore the showDetails check by: 1. Adding `const ctx = use()` to access the context 2. Adding a `shouldHide` memo with the original logic 3. Wrapping the return in `<Show when={!shouldHide()}>` The fix preserves the original behavior: - Hide completed tools when showDetails is false - Always show tools that are pending/running (so users see progress) - Always show tools that require permission (so users can respond)
## Problem The 'Hide tool details' toggle in the command palette (Ctrl+P) has no effect on the UI - completed tools are always displayed regardless of the setting. ## Root Cause The Permission rework commit 351ddee (merged 2026-01-01, PR anomalyco#6319) accidentally removed the showDetails check from the ToolPart function. ## Fix Restore the showDetails check with simplified logic: - Show all tools when showDetails is true - Show running/pending tools (so users see progress) - Hide completed tools when showDetails is false Removed the permission-based visibility check as the new PermissionPrompt dialog handles permissions separately at the bottom of the screen.
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
The permission rework in anomalyco#6319 removed the ability to override the default task tool restriction via agent frontmatter. Previously, agents could set `tools: { task: true }` to enable nested sub-agents. This fix restores that functionality using the new permission system: - Check if the agent's permission ruleset explicitly allows task tool - If so, don't add the deny rule to session permissions - If so, don't pass task: false to the tools parameter Agents can now enable nested sub-agents by setting: ```yaml permission: task: allow ``` The default behavior remains unchanged - task tool is disabled for sub-agents unless explicitly allowed.
The permission rework in anomalyco#6319 removed the ability to override the default task tool restriction via agent frontmatter. This fix checks if the agent has any task permission rule defined. If so, we don't add our default deny - letting the agent's permission take effect. If not specified, the default deny behavior is preserved.
…omalyco#6319) for tool availability and tool execution
…omalyco#6319) for tool availability and tool execution
Summary
This is a major change that overhauls the permissions system in OpenCode.
Tools Merged into Permission
The
toolsconfiguration has been deprecated and merged into thepermissionfield. Previously, you could enable/disable tools like this:{ "tools": { "bash": true, "edit": false } }Now, this should be configured using
permission:{ "permission": { "bash": "allow", "edit": "deny" } }The old
toolsconfig is still supported for backwards compatibility and will be automatically migrated to the permission system.Granular Permissions with Object Syntax
Permissions now support granular control using an object syntax with pattern matching. When you specify a permission as an object, you can set different rules for different patterns:
{ "permission": { "bash": { "npm *": "allow", "git *": "allow", "rm *": "deny", "*": "ask" }, "edit": { "*.md": "allow", "*.ts": "ask", "*": "deny" } } }Each key in the object is a glob pattern that matches against the tool's input, and the value is the action to take:
"allow"- automatically approve"deny"- automatically reject"ask"- prompt the user for approvalYou can also set a blanket permission using a simple string:
{ "permission": { "bash": "allow", "edit": "ask" } }Or set all permissions at once:
{ "permission": "allow" }Breaking Changes for SDK Users
The permission events have changed significantly. The new
PermissionNextmodule (permission/next.ts) has a different event structure compared to the oldPermissionmodule (permission/index.ts):Old Event Structure (
Permission.Event):Updated:{ id, type, pattern, sessionID, messageID, callID, message, metadata, time }Replied:{ sessionID, permissionID, response }New Event Structure (
PermissionNext.Event):Asked:{ id, sessionID, permission, patterns, metadata, always, tool: { messageID, callID } }Replied:{ sessionID, requestID, reply }Key differences:
permission.updatedtopermission.askedtyperenamed topermissionpatternis nowpatterns(array of strings)messagefield removedresponserenamed toreplypermissionIDrenamed torequestIDalwaysfield contains patterns that would be approved for future requests if user selects "always"The reply values are the same:
"once","always", or"reject"Server Changes
POST /permission/:requestID/replyfor responding to permission requestsPOST /session/:sessionID/permissions/:permissionIDis now deprecatedGET /permissionnow returnsPermissionNext.Request[]instead ofPermission.Info[]server.corsconfig optionOther Changes
toolsfield is deprecated - usepermissioninsteadmaxStepsis deprecated - usestepsinsteadoptions