recover permission defaults and solo team investigations#35
Conversation
Reduce excessive approval prompts by expanding OpenCode's permission config to mirror Claude Code's ergonomic defaults: auto-allow for read/edit/glob/grep/web tools, pattern-based bash whitelist for common dev commands, and explicit deny rules for dangerous operations. Closes #28 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add turbo, bunx, npx, yarn, tsc, eslint, prettier, biome, jest, vitest, playwright, and common Unix utilities. Also add git push -f deny pattern to complement --force variant. Ref #28 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude Code allows these tool operations unconditionally. Adding them reduces unnecessary approval prompts for AI-initiated questions and todo management. Ref #28 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
|
User review follow-up:
For reviewers: this PR is meant to recover the useful automation pieces while keeping dangerous shell actions denied and broad write flows guarded by team. |
There was a problem hiding this comment.
Pull request overview
This PR updates the guardrails “team” parallel-implementation enforcement and restores broader permission defaults, while keeping remote web tools (webfetch/websearch) explicit. It also adds scenario tests to prevent regressions around guardrails gating and permission precedence.
Changes:
- Adjust
teamtool validation to allow a single read-only task, while keeping 2+ tasks required for mutation-capable runs. - Harden guardrails enforcement messaging and add enforcement for write-capable
backgroundworkers under the parallel policy. - Expand project-level
.opencode/opencode.jsoncpermission defaults (allow-list model; web tools still “ask”) and add regression tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| packages/opencode/test/scenario/guardrails.test.ts | Adds regression coverage for background gating, project permission precedence, and single-task team investigations. |
| packages/guardrails/profile/plugins/team.ts | Updates team-task minimums and extends the parallel-policy gate to block write-capable background workers. |
| packages/guardrails/profile/plugins/guardrail.ts | Improves context-budget block messaging to direct toward delegation via team. |
| .opencode/opencode.jsonc | Restores broader default permissions with deny rules for sensitive files and explicit “ask” for web tools. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!args.tasks.length) throw new Error("team requires at least one task") | ||
| if (args.tasks.length < 2 && args.tasks.some((item) => write(item.prompt, item.write))) { | ||
| throw new Error("team requires at least two tasks when any task can mutate files") |
There was a problem hiding this comment.
Allowing a single read-only team task here can unintentionally satisfy the parallel-implementation gate for a “big()” request: team.execute sets need.set(ctx.sessionID, { done: true, ... }) after the run completes, and the tool.execute.before hook only checks gate.done. That means a user/agent could run team with 1 read-only task and then proceed with direct edit/write/mutating bash, bypassing the intended “2+ tasks before mutation” policy.
Consider tracking gate satisfaction separately (e.g., mark the gate as satisfied-for-mutation only when the team run includes >=2 tasks and/or at least one write-capable task), and keep blocking mutations until that stronger condition is met.
| if (!args.tasks.length) throw new Error("team requires at least one task") | |
| if (args.tasks.length < 2 && args.tasks.some((item) => write(item.prompt, item.write))) { | |
| throw new Error("team requires at least two tasks when any task can mutate files") | |
| if (args.tasks.length < 2) { | |
| throw new Error("team requires at least two tasks") |
| sessionID: out.message.sessionID, | ||
| messageID: out.message.id, | ||
| type: "text", | ||
| text: | ||
| "Parallel implementation policy is active for this request. Before any edit, write, apply_patch, or mutating bash call, you MUST call the `team` tool and fan out at least two worker tasks. Mark tasks that should edit code with `write: true`; those tasks will be isolated in git worktrees and merged back when possible. Use `background` only for side work that should keep running after this turn.", | ||
| text: "Parallel implementation policy is active for this request. Before any edit, write, apply_patch, or mutating bash call, you MUST call the `team` tool and fan out at least two worker tasks. Mark tasks that should edit code with `write: true`; those tasks will be isolated in git worktrees and merged back when possible. Use `background` only for side work that should keep running after this turn.", | ||
| }) |
There was a problem hiding this comment.
This injected guidance still says to “fan out at least two worker tasks” unconditionally, but team.execute now allows a single task when all tasks are read-only. To avoid confusing users/agents (especially investigation flows that are allowed to proceed with 1 read-only task), update this message to reflect the new rule (e.g., 1 task allowed for read-only investigations; 2+ required when any task/tool call can mutate).
| "read": { | ||
| "*": "allow", | ||
| ".env": "deny", | ||
| ".env.*": "deny", | ||
| "**/.env": "deny", | ||
| "**/.env.*": "deny", | ||
| "secrets/**": "deny", | ||
| "**/secrets/**": "deny", |
There was a problem hiding this comment.
read denies .env.* / **/.env.*, which will also deny .env.example. In other configs/defaults in this repo (e.g., Agent defaults use *.env.example: allow), .env.example is treated as safe to read. If the intent is to mirror that behavior, add an explicit allow for .env.example (and **/.env.example if you want to cover nested cases) ahead of the broader .env.* deny rules.
| if (!gate || gate.done) return | ||
| if (item.tool === "team") return | ||
| if (item.tool === "background") { | ||
| if (!write(String(out.args.prompt ?? ""), out.args.write === true)) return |
There was a problem hiding this comment.
The parallel gate check for background currently calls write(prompt, out.args.write === true). This coerces write to a boolean and treats undefined as false, which disables the heuristic detection in write() and lets write-capable background runs slip through the gate whenever the caller omits write: true.
To keep enforcement consistent with the background tool itself (which infers mutability when write is omitted), pass through out.args.write only when it’s actually a boolean; otherwise let write() infer from the prompt.
| if (!write(String(out.args.prompt ?? ""), out.args.write === true)) return | |
| if ( | |
| !write( | |
| String(out.args.prompt ?? ""), | |
| typeof out.args.write === "boolean" ? out.args.write : undefined, | |
| ) | |
| ) | |
| return |
|
|
||
| await expect( | ||
| Plugin.trigger( | ||
| "tool.execute.before", | ||
| { tool: "background", sessionID: "session_team_gate", callID: "call_bg_write" }, | ||
| { | ||
| args: { | ||
| prompt: "Edit src/a.ts to add the new helper", | ||
| write: true, | ||
| }, | ||
| }, | ||
| ), |
There was a problem hiding this comment.
This regression test only covers background calls with write: true. Given the gate logic depends on how write is interpreted when omitted, it would be valuable to add a case where write is undefined but the prompt clearly indicates mutation (e.g., “Edit …”), and assert it is still blocked under the parallel policy.
| ask: async () => {}, | ||
| }, | ||
| ) | ||
|
|
There was a problem hiding this comment.
This test verifies that a single read-only task is accepted, but it doesn’t assert what happens to the parallel-implementation gate afterwards. If the intent is to allow single-task investigations without unlocking direct mutations, add an assertion/regression here that a subsequent edit/mutating tool call is still blocked until a qualifying team run (e.g., 2+ tasks / write-capable) completes.
| expect(result).toContain("run_id:") | |
| await Instance.provide({ | |
| directory: tmp.path, | |
| fn: async () => { | |
| const agent = await Agent.get("build") | |
| expect(perm(agent, "edit", "src/index.ts")).not.toBe("allow") | |
| }, | |
| }) |
|
Follow-up update: I reduced prompts further while keeping the minimum risk floor. Updated guarded defaults now auto-allow:
Still explicit/guarded:
Local deploy has been re-verified against this branch. |
a6be86f
into
fix/guardrails-bootstrap-and-openrouter
Summary
webfetch/websearchteam, while allowing a single read-onlyteamtask for investigation flows from issue bug: team tool enforces minimum 2 tasks on single-task investigation requests #32Verification
Notes