fix: address marketplace review findings

src/agents/index.ts

@@ -146,6 +146,11 @@ export function normalizePermission(permission: unknown): PermissionRecord {
   return {};
 }
 
+/** A wildcard deny must remain authoritative under v2's last-match-wins rules. */
+function hasWildcardDeny(permission: PermissionRecord): boolean {
+  return permission['*'] === 'deny';
+}
+
 function mergePermissionRules(
   base: PermissionRecord,
   override: PermissionRecord,
@@ -303,7 +308,9 @@ export function projectAgentPermission(
   const permission = normalizePermission(registryEntry?.permission);
   const hostPermission = normalizePermission(hostEntry.permission);
   const projected = mergePermissionRules(permission, hostPermission);
-  applyTaskControlDefaults(canonicalName, projected);
+  if (!hasWildcardDeny(projected)) {
+    applyTaskControlDefaults(canonicalName, projected);
+  }
   if (canonicalName !== 'orchestrator') {
     projected.wait_for_user = 'deny';
     projected.marketplace = 'deny';
@@ -321,7 +328,9 @@ function projectPermissionValues(
     permission,
     normalizePermission(hostPermission),
   );
-  applyTaskControlDefaults(canonicalName, projected);
+  if (!hasWildcardDeny(projected)) {
+    applyTaskControlDefaults(canonicalName, projected);
+  }
   if (canonicalName !== 'orchestrator') {
     projected.wait_for_user = 'deny';
     projected.marketplace = 'deny';
@@ -707,6 +716,12 @@ function applyDefaultPermissionPolicy(
   disabledSkills?: readonly string[],
 ): void {
   const existing = normalizePermission(agent.config.permission);
+  // A user/package deny-all intentionally disables every capability. Adding
+  // role, question, or skill allows after it reopens those tools on v2.
+  if (hasWildcardDeny(existing)) {
+    agent.config.permission = existing as SDKAgentConfig['permission'];
+    return;
+  }
   const role = agent.baseRole ? ROLE_DEFINITIONS[agent.baseRole] : undefined;
 
   // Get skill-specific permissions for this agent
@@ -750,6 +765,10 @@ function applyDefaultPermissionPolicy(
 /** Task controls are editable defaults, not immutable gates. */
 function applyDefaultTaskControls(agent: AgentDefinition): void {
   const permission = normalizePermission(agent.config.permission);
+  if (hasWildcardDeny(permission)) {
+    agent.config.permission = permission as SDKAgentConfig['permission'];
+    return;
+  }
   const canonicalName = agent.baseRole ?? agent.name;
   applyTaskControlDefaults(canonicalName, permission);
   agent.config.permission = {
@@ -806,6 +825,10 @@ function applyMarketplaceCapabilities(
   activated: ActivatedMarketplaceAgent | ActivatedMarketplaceProfile,
 ): void {
   const permission = normalizePermission(agent.config.permission);
+  if (hasWildcardDeny(permission)) {
+    agent.config.permission = permission as SDKAgentConfig['permission'];
+    return;
+  }
   const role = agent.baseRole ? ROLE_DEFINITIONS[agent.baseRole] : undefined;
   for (const tool of activated.manifest.capabilities.tools) {
     permission[tool] = 'allow';
@@ -1578,6 +1601,7 @@ function applyMcpPermissionRules(
   availableMcpNames: readonly string[],
 ): PermissionRecord {
   const result = normalizePermission(permission);
+  if (hasWildcardDeny(result)) return result;
   const denied = new Set(
     agentMcps
       .filter((name) => name.startsWith('!'))

src/agents/registry.test.ts

@@ -23,6 +23,33 @@ function registryFor(config: PluginConfig = {}): ResolvedAgentRegistry {
 }
 
 describe('ResolvedAgentRegistry', () => {
+  test('keeps deny-all authoritative for a role-derived agent under v2 matching', () => {
+    const registry = registryFor({
+      agents: {
+        audit: {
+          baseRole: 'fixer',
+          model: 'provider/audit',
+          permission: 'deny',
+        },
+      },
+    });
+    const permission = registry.sdkConfigs.audit?.permission as Record<
+      string,
+      unknown
+    >;
+
+    // v2 resolves an explicit tool rule after the wildcard, so any generated
+    // allow would reopen that tool. Only immutable deny gates may be added.
+    const v2EffectivePermission = (tool: string): unknown =>
+      permission[tool] ?? permission['*'];
+    expect(v2EffectivePermission('read')).toBe('deny');
+    expect(v2EffectivePermission('edit')).toBe('deny');
+    expect(v2EffectivePermission('question')).toBe('deny');
+    expect(v2EffectivePermission('task_cancel')).toBe('deny');
+    expect(permission.wait_for_user).toBe('deny');
+    expect(permission.marketplace).toBe('deny');
+  });
+
   test('keeps SDK, model, skill, MCP, and routing surfaces consistent', () => {
     const registry = registryFor({
       agents: {

src/cli/config-io.test.ts

@@ -135,6 +135,27 @@ describe('config-io', () => {
     expect(existsSync(`${path}.lock`)).toBe(false);
   });
 
+  test('mutateJsonFile warns before replacing JSONC comments', () => {
+    const path = join(tmpDir, 'mutate.jsonc');
+    writeFileSync(path, '{\n  // retain this manually\n  "count": 1\n}\n');
+    const warn = mock(() => {});
+    const originalWarn = console.warn;
+    console.warn = warn;
+
+    try {
+      mutateJsonFile(path, (current) => ({
+        ...(current as Record<string, unknown>),
+        count: 2,
+      }));
+    } finally {
+      console.warn = originalWarn;
+    }
+
+    expect(warn).toHaveBeenCalledWith(
+      '[config-manager] Writing to .jsonc file - comments will not be preserved',
+    );
+  });
+
   test('writeConfig writes JSON and creates backup', () => {
     const path = join(tmpDir, 'test.json');
     writeFileSync(path, '{"old": true}');

src/cli/config-io.ts

@@ -498,6 +498,11 @@ export function mutateJsonFile(
   mutate: (current: unknown) => unknown,
 ): void {
   withSerializedConfigWrite(filePath, () => {
+    if (filePath.endsWith('.jsonc')) {
+      console.warn(
+        '[config-manager] Writing to .jsonc file - comments will not be preserved',
+      );
+    }
     const original = readFileBytes(filePath);
     const parsed =
       original === null

src/config/loader.test.ts

@@ -109,6 +109,29 @@ describe('loadPluginConfig', () => {
     expect(config.agents?.oracle?.model).toBe('test/model');
   });
 
+  test('normalizes flat legacy presets without dropping unrelated config', () => {
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({
+        preset: 'fast',
+        disabled_tools: ['websearch'],
+        presets: {
+          fast: { explorer: { model: 'legacy/explorer' } },
+        },
+      }),
+    );
+
+    const config = loadPluginConfig(projectDir);
+
+    expect(config.disabled_tools).toEqual(['websearch']);
+    expect(config.presets?.fast?.agents.explorer?.model).toBe(
+      'legacy/explorer',
+    );
+  });
+
   test('loads autoUpdate flag when configured', () => {
     const projectDir = path.join(tempDir, 'project');
     const projectConfigDir = path.join(projectDir, '.opencode');

src/config/runtime.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, test } from 'bun:test';
 import { RuntimeConfig } from './runtime';
-import type { PluginConfig } from './schema';
+import { type PluginConfig, PluginConfigSchema } from './schema';
 
 const DIRECTORY = '/tmp/runtime-config-test';
 
@@ -34,6 +34,22 @@ describe('RuntimeConfig', () => {
     expect(runtime.agent('explorer')?.model).toBe('original');
   });
 
+  test('resolves normalized legacy preset agents through the single preset path', () => {
+    resetRegistry();
+    const plugin = PluginConfigSchema.parse({
+      preset: 'legacy',
+      presets: {
+        legacy: { explorer: { model: 'legacy/explorer' } },
+      },
+    });
+    const runtime = RuntimeConfig.init(DIRECTORY, plugin);
+
+    expect(runtime.agent('explorer')?.model).toBe('legacy/explorer');
+    expect(runtime.plugin?.presets?.legacy).toEqual({
+      agents: { explorer: { model: 'legacy/explorer' } },
+    });
+  });
+
   test('seed precedence: host override > root > selected preset', () => {
     resetRegistry();
     const plugin: PluginConfig = {

src/config/schema.test.ts

@@ -22,11 +22,29 @@ describe('structured preset schema', () => {
     }
   });
 
-  it('rejects the obsolete flat preset agent map', () => {
-    expect(
-      PresetSchema.safeParse({ explorer: { model: 'provider/explorer' } })
-        .success,
-    ).toBe(false);
+  it('normalizes the legacy flat preset agent map', () => {
+    const result = PresetSchema.safeParse({
+      explorer: { model: 'provider/explorer' },
+    });
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data).toEqual({
+        agents: { explorer: { model: 'provider/explorer' } },
+      });
+    }
+  });
+
+  it('gives structured agent overrides precedence in mixed presets', () => {
+    const result = PresetSchema.safeParse({
+      explorer: { model: 'legacy/explorer' },
+      agents: { explorer: { model: 'structured/explorer' } },
+      marketplace: { agents: ['community/example'] },
+    });
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data.agents.explorer.model).toBe('structured/explorer');
+      expect(result.data.marketplace?.agents).toEqual(['community/example']);
+    }
   });
 
   it('normalizes package IDs and validates profile targets', () => {

src/config/schema.ts

@@ -214,12 +214,40 @@ export const MarketplaceActivationSchema = z
 
 export type MarketplaceActivation = z.infer<typeof MarketplaceActivationSchema>;
 
-export const PresetSchema = z
-  .object({
-    agents: z.record(z.string(), AgentOverrideConfigSchema).default({}),
-    marketplace: MarketplaceActivationSchema.optional(),
-  })
-  .strict();
+function normalizeLegacyPreset(value: unknown): unknown {
+  if (typeof value !== 'object' || value === null || Array.isArray(value)) {
+    return value;
+  }
+
+  const preset = value as Record<string, unknown>;
+  const { agents: structuredAgents, marketplace, ...legacyAgents } = preset;
+  if (Object.keys(legacyAgents).length === 0) return value;
+
+  // v3's explicit keys win when a mixed layout contains the same agent.
+  // Normalizing at the schema boundary keeps all runtime consumers on the
+  // structured representation.
+  return {
+    ...(marketplace === undefined ? {} : { marketplace }),
+    agents: {
+      ...legacyAgents,
+      ...(structuredAgents &&
+      typeof structuredAgents === 'object' &&
+      !Array.isArray(structuredAgents)
+        ? structuredAgents
+        : {}),
+    },
+  };
+}
+
+export const PresetSchema = z.preprocess(
+  normalizeLegacyPreset,
+  z
+    .object({
+      agents: z.record(z.string(), AgentOverrideConfigSchema).default({}),
+      marketplace: MarketplaceActivationSchema.optional(),
+    })
+    .strict(),
+);
 
 export type Preset = z.infer<typeof PresetSchema>;