Przeglądaj źródła

Fix .catchall() and handle shorthand string permission

Vinaykumar Pandya 1 miesiąc temu
rodzic
commit
e5de21dadf

+ 52 - 2
oh-my-opencode-slim.schema.json

@@ -426,7 +426,32 @@
                       ]
                     }
                   },
-                  "additionalProperties": {}
+                  "additionalProperties": {
+                    "anyOf": [
+                      {
+                        "type": "string",
+                        "enum": [
+                          "ask",
+                          "allow",
+                          "deny"
+                        ]
+                      },
+                      {
+                        "type": "object",
+                        "propertyNames": {
+                          "type": "string"
+                        },
+                        "additionalProperties": {
+                          "type": "string",
+                          "enum": [
+                            "ask",
+                            "allow",
+                            "deny"
+                          ]
+                        }
+                      }
+                    ]
+                  }
                 }
               ]
             }
@@ -836,7 +861,32 @@
                     ]
                   }
                 },
-                "additionalProperties": {}
+                "additionalProperties": {
+                  "anyOf": [
+                    {
+                      "type": "string",
+                      "enum": [
+                        "ask",
+                        "allow",
+                        "deny"
+                      ]
+                    },
+                    {
+                      "type": "object",
+                      "propertyNames": {
+                        "type": "string"
+                      },
+                      "additionalProperties": {
+                        "type": "string",
+                        "enum": [
+                          "ask",
+                          "allow",
+                          "deny"
+                        ]
+                      }
+                    }
+                  ]
+                }
               }
             ]
           }

+ 52 - 0
src/agents/custom.test.ts

@@ -452,3 +452,55 @@ describe('custom-agent permission passthrough', () => {
     ).toBeUndefined();
   });
 });
+
+describe('permission edge cases', () => {
+  test('shorthand string permission is not corrupted by applyDefaultPermissions', () => {
+    const config: PluginConfig = {
+      agents: {
+        planner: {
+          model: 'openai/gpt-5.5',
+          permission: 'ask',
+        },
+      },
+    };
+
+    const agents = createAgents(config);
+    const planner = agents.find((a) => a.name === 'planner');
+
+    expect(planner).toBeDefined();
+    // The shorthand string should be preserved as-is, not spread into
+    // character keys like { "0": "a", "1": "s", "2": "k" }
+    expect(planner?.config.permission).toBe('ask');
+  });
+
+  test('orchestrator permission override does not replace plugin gates', () => {
+    const config: PluginConfig = {
+      agents: {
+        orchestrator: {
+          model: 'openai/gpt-5.5',
+          permission: { edit: 'deny' },
+        },
+      },
+    };
+
+    const agents = createAgents(config);
+    const orchestrator = agents.find((a) => a.name === 'orchestrator');
+
+    expect(orchestrator).toBeDefined();
+    // User-supplied key survives
+    expect(orchestrator?.config.permission).toMatchObject({
+      edit: 'deny',
+    });
+    // Plugin-generated gates are NOT dropped by the override
+    expect(
+      (orchestrator?.config.permission as Record<string, unknown>)?.question,
+    ).toBeDefined();
+    expect(
+      (orchestrator?.config.permission as Record<string, unknown>)
+        ?.council_session,
+    ).toBeDefined();
+    expect(
+      (orchestrator?.config.permission as Record<string, unknown>)?.cancel_task,
+    ).toBeDefined();
+  });
+});

+ 10 - 0
src/agents/index.test.ts

@@ -1107,4 +1107,14 @@ describe('AgentOverrideConfigSchema permission validation', () => {
     });
     expect(result.success).toBe(false);
   });
+
+  test('rejects array value for unknown permission key', () => {
+    const result = AgentOverrideConfigSchema.safeParse({
+      model: 'openai/gpt-5.5',
+      permission: {
+        custom_tool: ['foo', 'bar'],
+      },
+    });
+    expect(result.success).toBe(false);
+  });
 });

+ 10 - 3
src/agents/index.ts

@@ -275,6 +275,13 @@ function applyDefaultPermissions(
   configuredSkills?: string[],
   disabledSkills?: string[],
 ): void {
+  // If the user supplied a shorthand string permission (e.g. "ask"),
+  // it already applies to all tools — preserve it as-is and skip the
+  // object merge, which would corrupt it by spreading the string.
+  if (typeof agent.config.permission === 'string') {
+    return;
+  }
+
   const existing = (agent.config.permission ?? {}) as Record<
     string,
     'ask' | 'allow' | 'deny' | Record<string, 'ask' | 'allow' | 'deny'>
@@ -539,14 +546,14 @@ export function createAgents(
     orchestratorPrompts.appendPrompt,
   );
 
+  if (orchestratorOverride) {
+    applyOverrides(orchestrator, orchestratorOverride);
+  }
   applyDefaultPermissions(
     orchestrator,
     orchestratorOverride?.skills,
     config?.disabled_skills,
   );
-  if (orchestratorOverride) {
-    applyOverrides(orchestrator, orchestratorOverride);
-  }
 
   // Collect all display names from orchestrator and all subagents
   const displayNameMap = new Map<string, string>();

+ 2 - 2
src/config/schema.ts

@@ -66,7 +66,7 @@ const PermissionRuleSchema = z.union([
 ]);
 
 // Known keys are typed for typo protection; .catchall() types the index
-// signature to match the opencode SDK's PermissionConfig, so no cast is needed at
+// signature to match the SDK's PermissionConfig, so no cast is needed at
 // the assignment site. Unknown tool keys are still validated as rules.
 const PermissionObjectSchema = z
   .object({
@@ -87,7 +87,7 @@ const PermissionObjectSchema = z
     codesearch: PermissionActionSchema.optional(),
     doom_loop: PermissionActionSchema.optional(),
   })
-  .catchall(z.union([PermissionRuleSchema, z.array(z.string())]));
+  .catchall(PermissionRuleSchema);
 
 export const PermissionConfigSchema = z.union([
   PermissionActionSchema,