Repository navigation
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The V1 ruleset now resolves from ordered permission documents ( Files Reviewed (6 files)
Previous Review Summaries (8 snapshots, latest commit fe20f2b)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit fe20f2b)Status: No Issues Found | Recommendation: Merge The incremental change adds a temp global config directory and redirects Files Reviewed (1 file)
Previous review (commit 0c80210)Status: 1 Issue Found | Recommendation: Address before merge The incremental change only removes the unused global Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous review (commit f830abd)Status: 1 Issue Found | Recommendation: Address before merge The incremental change only retargets the overlay refresh test to a project-only permission key, which is the correct adjustment for the new global-wins precedence. One test-quality nit remains: the global fixture is now inert and the baseline assertion passes on the built-in default. Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file)
Previous review (commit c70240e)Status: No Issues Found | Recommendation: Merge The previous warning about legacy Files Reviewed (2 files)
Previous review (commit 2b3cebf)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 2864313)Status: No Issues Found | Recommendation: Merge The incremental commit Files Reviewed (3 files)
Previous review (commit acdbc11)Status: 2 Issues Found | Recommendation: Address before merge Overview
The incremental commit ( Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit b974f7b)Status: 2 Issues Found | Recommendation: Address before merge Overview
The ordering change itself looks correct: Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Reviewed by deepseek-flash-latest · Input: 145.2K · Output: 40.8K · Cached: 4.9M Review guidance: REVIEW.md from base branch |
|
Changes requested: the V1 precedence fix can still be bypassed through key order.
|
Build the v1 ruleset from config documents in ascending precedence instead of flattening the merged permission map, and apply the same order before the markdown source guard reads permission rules. Multi-pattern entries in a shared key now resolve with the global rule last.
|
bot: Addressed in 183d107.
|
Evidence
Evidence: demonstrated — the change was exercised on the work VM. The
codeagent ruleset was resolved through real config discovery with a global document and a repository document that both name the samebashpattern, and then used to gate a real tool call: the pattern-specific command is refused by the permission check and does not execute, while a command covered by the global entry runs and returns its output.debug configprints the ordered document layers and the merged map. Unit tests cover the same resolution for V1 and V2, including the markdown source guard.What changed
packages/core/src/config/plugin/agent.tsapplies agent permission rules from configuration documents in reverse document priority: repository-scoped documents first, the user-global document last. Permission resolution keeps the last matching rule, so a global rule defines the result when a global and a repository document both configure the same action and resource.default_agent,model,variant,request,system,description,mode,hidden,color,steps,disabled) keep their nearest-wins merge, in a separate pass over the same documents.permissionsarray is applied before that document'sagents[<id>].permissions, and statement order inside each array is unchanged.agentplugin runs first.packages/opencode/src/config/config.tsapplies the same ordering to the V1permissionconfiguration that CLI agent resolution reads:permissionentries are collected as separate documents, ordered project first and global last, and exposed as the derivedpermission_layersfield alongside the merged map.packages/opencode/src/permission/index.tsgainsfromScopes, which flattens those documents in order, andpackages/opencode/src/agent/agent.tsbuilds the configured ruleset with it. A key that carries several patterns therefore resolves each pattern by document priority instead of by the position the key first appeared at in the merged map.permission_origins, the config overlay, and config reads agree with the ruleset. The derived field is stripped from persisted and editable config.markdown_sourceentry decides a root that a repository document also names..kilocodeignoremigration joins the project side, so a projectpermissionentry still applies after the migrated ignore rules.Policy
The ordering matches what
packages/core/src/config.tsalready does forexperimental.policies.packages/core/src/permission.ts(V2) andpackages/opencode/src/permission/index.ts(V1) both resolve with the last matching rule, so the ruleset is built in document order rather than from a merged map whose key order no longer carries precedence.Verification
packages/core:bun run typecheckclean;bun test ./test/config/agent.test.ts→ 8 pass, including a globalallowwith a pattern-specificdenyand a repository document that names the same pattern asallow(resolved effect:deny), and a command the global entry allows (resolved effect:allow).packages/opencode:bun run typecheckclean;bun run script/test-runner.ts permission-origins config-overlay→ 2 files, 2 passed. The permission tests cover a global multi-patternbashentry with a repository entry for the same key and pattern (global effect wins), a repository entry for a key the global document does not mention (still applies), and a globalmarkdown_sourceentry with a repository entry for the same root (the project agent document is not loaded from that root).bashtoallowwith a pattern-specificdeny, the repository document names that pattern asallow, anddebug agent code --tool bashrefuses the pattern-specific command with the permission error without executing it, while a command the global entry allows runs and returns1.4.2.debug configprintspermission_layersas[project, global]and the mergedbashentry with both patterns.bun run lint: 0 errors.script/check-opencode-annotations.ts --worktreereports all shared upstream changes annotated.check:duplication,check-workflows, andcheck-md-table-paddingpass.Production prerequisites
None.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Reverse document order
Confidence: 0.85 - Precedent: least work with no quality drop; matching the policy loader's user-global-over-repository ordering under last-match resolution avoids a second evaluation mode.
agents[id].permissions(agent-level rules), or only top-levelpermissions?Answer: Both, same reversal
Confidence: 0.85 - Precedent: fix the class on every affected surface, and the brief explicitly covers agents defined in config.
Answer: Yes, neutral note
Confidence: 0.65 - Precedent: update docs that would otherwise be incomplete, worded neutrally.
permissionmap thatpackages/opencode/src/config/config.tsmerges. Should this PR also reorder the V1 path, or keep the V2 fix and disclose the V1 behavior?Answer: Extend this PR to V1 too
Confidence: 0.80 - Precedent: fix the root cause and the class on every affected surface, and the brief asks for any separate CLI path with the same ordering to be made consistent.