Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes the reported bug, but it also takes away the documented way to let Plan edit files, so I think it needs one change first.
I ran the reproduction from #55127 from source on both checkouts (global permission.edit: { "*": "allow" }, switch to Plan, have the simulated model patch src/value.ts). On the base the file is modified. With this change the patch is refused with "Cannot use patch to modify files outside the Plan directory" and the file stays the same. bun test test/plugin/plan.test.ts passes (13 tests) and typecheck in packages/core is clean.
Public API: unchanged.
Concerns:
- Plan can no longer be configured to edit. I ran a second check with no global rule and only a per-agent rule,
agents.plan.permissions: [{ action: "edit", resource: "*", effect: "allow" }]. On the base Plan edits the file; with this change the edit is refused.permissions.mdxsays agent rules take precedence over the global config. The hook also turns a configurededit: askintodeny. Details are in the inline comment. - The test only checks the hook on its own. It calls the hook directly with a made-up permission check, so it never goes through the path where config rules are added and can't catch the regression above. A test that loads a config with a global
edit: allowand checks the Plan agent's real result would cover the actual bug. Ideally it would also check that a per-agentagents.planallow still wins.
Related: #39715, #55683, #46358, #55814
| // last match, so a broad `edit: allow` from user config would silently defeat Plan mode. | ||
| // Enforce the read-only boundary after the configured effect has been computed. | ||
| const allowed = path.join(directory, "*") | ||
| yield* ctx.permission.hook("evaluate", (event) => |
There was a problem hiding this comment.
This hook ignores the rules and turns every non-deny edit result for Plan into deny. So an explicit agents.plan.permissions edit allow, an ask, and session-level rules are all overridden. I checked the per-agent case: with only agents.plan.permissions: [{ action: "edit", resource: "*", effect: "allow" }], the base lets Plan edit and this change refuses it. The documented way to let Plan edit no longer works.
The bug comes from rule order. config/plugin/agent.ts adds the top-level config rules after each built-in agent's own rules (agent.permissions.push(...permissions)). Putting those global rules first, while per-agent agents.<id>.permissions stay last, would fix #55127 and keep overrides working. It would also fix built-ins like explore (* deny), which a global allow currently overrides the same way. That change would also make this extra hook and its second hard-coded plan path unnecessary.
|
Thanks for picking this up, @Finn763. We're closing this. We decided the behavior in #55127 is expected: top-level We fixed the docs instead in #56922. The Plan section now describes the actual defaults and shows how to keep Plan read-only with per-agent permissions. |
Issue for this PR
Closes #55127
Type of change
What does this PR do?
Configured permissions are appended to every agent after the built-in plugin rules, and
Permission.evaluatetakes the last matching rule. A global or project"permission": { "edit": { "*": "allow" } }therefore lands after the Plan agent'sedit: denyand silently turns Plan mode into Build mode for file edits.The plan plugin now also enforces its boundary through the existing
permission.evaluatehook: for the Plan agent, anediteffect that resolved toallowis downgraded todenywhen any resource is outside~/.opencode/plan/. This runs after the configured rules regardless of their order, and leaves other agents and in-directory edits untouched.How did you verify your code works?
keeps the read-only boundary when configured permissions append a broad allowcovers outside → deny, plan-directory → allow, other agent → allow. It fails on base (no permission hook registered) and passes with the change.bun test test/plugin/plan.test.ts— 13 pass.bun run typecheckclean.Screenshots / recordings
N/A.
Checklist
Added by the opencode bot (AI-generated).
Related: #39715, #55683, #46358, #55814