fix: proper OpenCode v1.1.1 permission migration #490
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
….1.1 - Rewrite permission-compat.ts with runtime version detection - createAgentToolRestrictions() returns correct format per version - v1.1.1+ uses permission format, older uses tools format - Add migrateToolsToPermission/migratePermissionToTools helpers - Update test suite for new API 🤖 Generated with assistance of OhMyOpenCode https://github.com/code-yeongyu/oh-my-opencode
- Replace hardcoded tools: { X: false } format with version-aware utility
- All agents now use createAgentToolRestrictions([...])
- Ensures compatibility with both old and new OpenCode versions
🤖 Generated with assistance of OhMyOpenCode
https://github.com/code-yeongyu/oh-my-opencode
Migrate tools/permission format in user/project/plugin agent configs based on detected OpenCode version at load time. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Greptile SummaryThis PR properly implements OpenCode v1.1.1 permission system migration by fixing the incomplete approach from PR #489. The changes correctly apply version detection at runtime and migrate all agent configurations across the system. Key improvements:
What was wrong in #489: The previous PR created utilities but didn't properly integrate them - agents had mixed Migration logic: // v1.1.1+ detected
{ tools: { write: false } } → { permission: { write: "deny" } }
// Older version detected
{ permission: { write: "deny" } } → { tools: { write: false } }The implementation is clean, well-tested, and ensures both forward and backward compatibility across OpenCode versions. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Plugin as Plugin Init
participant ConfigHandler as config-handler.ts
participant VersionDetect as opencode-version.ts
participant PermCompat as permission-compat.ts
participant Agents as Agent Configs
Note over Plugin,Agents: Startup: Version Detection
Plugin->>ConfigHandler: Load configuration
ConfigHandler->>VersionDetect: getOpenCodeVersion()
VersionDetect->>VersionDetect: execSync("opencode --version")
VersionDetect-->>VersionDetect: Cache result
Note over Plugin,Agents: Builtin Agent Creation
ConfigHandler->>Agents: createBuiltinAgents()
Agents->>PermCompat: createAgentToolRestrictions(denyTools)
PermCompat->>VersionDetect: supportsNewPermissionSystem()
alt v1.1.1+
PermCompat-->>Agents: { permission: { tool: "deny" } }
else Older version
PermCompat-->>Agents: { tools: { tool: false } }
end
Note over Plugin,Agents: User Agent Migration
ConfigHandler->>ConfigHandler: Load user agents (oh-my-opencode.json)
ConfigHandler->>PermCompat: migrateAgentConfig(userAgent)
PermCompat->>VersionDetect: supportsNewPermissionSystem()
alt v1.1.1+ with tools config
PermCompat->>PermCompat: migrateToolsToPermission()
PermCompat-->>ConfigHandler: { permission: {...}, tools: deleted }
else Older with permission config
PermCompat->>PermCompat: migratePermissionToTools()
PermCompat-->>ConfigHandler: { tools: {...}, permission: deleted }
end
Note over Plugin,Agents: Project & Plugin Agent Migration
ConfigHandler->>PermCompat: migrateAgentConfig(projectAgent)
ConfigHandler->>PermCompat: migrateAgentConfig(pluginAgent)
ConfigHandler->>PermCompat: migrateAgentConfig(opencodeAgent)
Note over Plugin,Agents: Final Config Assembly
ConfigHandler-->>Plugin: Complete config with migrated agents
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 9 files
Confidence score: 3/5
src/plugin-handlers/config-handler.tscan reintroduce the legacytoolsfields after migration, so agent configs might silently revert to an old format and undo the intended schema upgrade.src/shared/permission-compat.tsstill uses a loosetypeof === "object"check, meaning arrays pass through and get migrated incorrectly (e.g.,['edit']becoming{ '0': 'allow' }), which could break permission handling.- Pay close attention to
src/plugin-handlers/config-handler.ts,src/shared/permission-compat.ts- migration logic needs to stay consistent and reject arrays to avoid regressions.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/shared/permission-compat.ts">
<violation number="1" location="src/shared/permission-compat.ts:58">
P2: Use `isPlainObject` from `./deep-merge` instead of `typeof === "object"` check. The current check passes for arrays, which would cause incorrect migration (e.g., `["edit"]` → `{ "0": "allow" }`).</violation>
<violation number="2" location="src/shared/permission-compat.ts:68">
P2: Use `isPlainObject` from `./deep-merge` instead of `typeof === "object"` check. This ensures arrays are properly rejected and maintains consistency with other object checks in the codebase.</violation>
</file>
<file name="src/plugin-handlers/config-handler.ts">
<violation number="1" location="src/plugin-handlers/config-handler.ts:110">
P1: After migrating agent configs, this handler later re-adds `tools` for `explore`/`librarian`/`multimodal-looker`, which can undo the migration and reintroduce legacy fields. Consider applying the same version-aware restriction logic here (set `permission` instead of `tools` when the new system is detected), or run migration after these mutations so the final shape is consistent.</violation>
<violation number="2" location="src/plugin-handlers/config-handler.ts:195">
P2: Migration isn’t applied to `config.agent` overrides when Sisyphus is disabled (`...configAgent` is merged as-is). This can leave old-format agent configs un-migrated in that mode. Consider migrating `configAgent` in the else-branch too (e.g., map entries through `migrateAgentConfig`) so version compatibility is consistent regardless of Sisyphus enablement.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| delete result.tools | ||
| } | ||
| } else { | ||
| if (result.permission && typeof result.permission === "object") { |
There was a problem hiding this comment.
P2: Use isPlainObject from ./deep-merge instead of typeof === "object" check. This ensures arrays are properly rejected and maintains consistency with other object checks in the codebase.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/permission-compat.ts, line 68:
<comment>Use `isPlainObject` from `./deep-merge` instead of `typeof === "object"` check. This ensures arrays are properly rejected and maintains consistency with other object checks in the codebase.</comment>
<file context>
@@ -1,72 +1,78 @@
+ delete result.tools
+ }
+ } else {
+ if (result.permission && typeof result.permission === "object") {
+ const existingTools = (result.tools as Record<string, boolean>) || {}
+ const migratedTools = migratePermissionToTools(
</file context>
| if (config.permission) { | ||
| result.permission = config.permission | ||
| if (supportsNewPermissionSystem()) { | ||
| if (result.tools && typeof result.tools === "object") { |
There was a problem hiding this comment.
P2: Use isPlainObject from ./deep-merge instead of typeof === "object" check. The current check passes for arrays, which would cause incorrect migration (e.g., ["edit"] → { "0": "allow" }).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/permission-compat.ts, line 58:
<comment>Use `isPlainObject` from `./deep-merge` instead of `typeof === "object"` check. The current check passes for arrays, which would cause incorrect migration (e.g., `["edit"]` → `{ "0": "allow" }`).</comment>
<file context>
@@ -1,72 +1,78 @@
- if (config.permission) {
- result.permission = config.permission
+ if (supportsNewPermissionSystem()) {
+ if (result.tools && typeof result.tools === "object") {
+ const existingPermission =
+ (result.permission as Record<string, PermissionValue>) || {}
</file context>
| : {}; | ||
| .map(([key, value]) => [ | ||
| key, | ||
| value ? migrateAgentConfig(value as Record<string, unknown>) : value, |
There was a problem hiding this comment.
P2: Migration isn’t applied to config.agent overrides when Sisyphus is disabled (...configAgent is merged as-is). This can leave old-format agent configs un-migrated in that mode. Consider migrating configAgent in the else-branch too (e.g., map entries through migrateAgentConfig) so version compatibility is consistent regardless of Sisyphus enablement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/plugin-handlers/config-handler.ts, line 195:
<comment>Migration isn’t applied to `config.agent` overrides when Sisyphus is disabled (`...configAgent` is merged as-is). This can leave old-format agent configs un-migrated in that mode. Consider migrating `configAgent` in the else-branch too (e.g., map entries through `migrateAgentConfig`) so version compatibility is consistent regardless of Sisyphus enablement.</comment>
<file context>
@@ -162,15 +182,20 @@ export function createConfigHandler(deps: ConfigHandlerDeps) {
- : {};
+ .map(([key, value]) => [
+ key,
+ value ? migrateAgentConfig(value as Record<string, unknown>) : value,
+ ])
+ )
</file context>
| const userAgents = Object.fromEntries( | ||
| Object.entries(rawUserAgents).map(([k, v]) => [ | ||
| k, | ||
| v ? migrateAgentConfig(v as Record<string, unknown>) : v, |
There was a problem hiding this comment.
P1: After migrating agent configs, this handler later re-adds tools for explore/librarian/multimodal-looker, which can undo the migration and reintroduce legacy fields. Consider applying the same version-aware restriction logic here (set permission instead of tools when the new system is detected), or run migration after these mutations so the final shape is consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/plugin-handlers/config-handler.ts, line 110:
<comment>After migrating agent configs, this handler later re-adds `tools` for `explore`/`librarian`/`multimodal-looker`, which can undo the migration and reintroduce legacy fields. Consider applying the same version-aware restriction logic here (set `permission` instead of `tools` when the new system is detected), or run migration after these mutations so the final shape is consistent.</comment>
<file context>
@@ -95,13 +96,32 @@ export function createConfigHandler(deps: ConfigHandlerDeps) {
+ const userAgents = Object.fromEntries(
+ Object.entries(rawUserAgents).map(([k, v]) => [
+ k,
+ v ? migrateAgentConfig(v as Record<string, unknown>) : v,
+ ])
+ );
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the actual v1.1.1 permission compatibility issue by implementing runtime version detection and automatic migration.
What was wrong before
Previous PR #489 created utilities but didn't properly use them:
toolsandpermissionin same config (invalid)What this PR does
Version-aware agent restrictions (
createAgentToolRestrictions){ permission: { tool: "deny" } }{ tools: { tool: false } }All builtin agents updated
createAgentToolRestrictions()instead of hardcoded formatsRuntime migration in config-handler.ts
oh-my-opencode.jsonMigration logic
Testing
Closes #487
🤖 100% written by Sisyphus
Generated with OhMyOpenCode
Summary by cubic
Fixes permission compatibility for OpenCode v1.1.1 by adding runtime version detection and automatic config migration. Updates all built-in agents to a version-aware restrictions helper to prevent mixed tools/permission formats.
Written for commit 6c3ef65. Summary will update on new commits.