fix(config): skills_add must not override inherited skill exclusions

docs/skills.md

@@ -334,4 +334,5 @@ Control which skills each agent can use in `~/.config/opencode/oh-my-opencode-sl
 - Duplicates are removed (first occurrence wins) before removals are applied
 - If the result contains `"*"`, each removed name is appended as `"!<name>"` so the exclusion beats the wildcard grant
 - On an agent without a `skills` list, directives resolve against that agent's default grants (orchestrator: all skills), so `skills_add` keeps the defaults and appends, and `skills_remove` prunes from them
+- `skills_add` never overrides an inherited exclusion: adding a name that the effective list already excludes (as `"!<name>"`) is a no-op. To re-allow it, lift the exclusion with a `skills_remove` entry of the form `"!<name>"`
 - A removal entry of the form `"!<name>"` removes the exclusion token itself (it lifts an existing exclusion); the `"*"` token is never expanded

src/cli/skills.ts

@@ -106,8 +106,11 @@ export function getSkillPermissionsForAgent(
  *    grants (orchestrator defaults to allow-all, so its working base is
  *    `['*']`), so `skills_add` alone keeps the defaults and appends, and
  *    `skills_remove` alone prunes from the defaults.
- * 2. The working base and `add` are concatenated, deduped (first
- *    occurrence wins), then removal entries are filtered out.
+ * 2. The working base and `add` are concatenated and deduped (first
+ *    occurrence wins). Additions matching an exclusion (`'!name'`) token
+ *    in the base are dropped, because the resolver applies tokens in order
+ *    and a later plain grant would override the inherited deny. Then
+ *    removal entries are filtered out.
  * 3. Removals operate on final skill tokens, never expanding `'*'`: a
  *    plain name removes that name token, and a `'!name'` entry removes
  *    the exclusion token itself (lifting an existing exclusion). If the
@@ -143,11 +146,23 @@ export function resolveEffectiveSkills(
       ? ['*']
       : getDefaultGrantedSkillNames(agentName));
 
+  // Exclusion tokens in the base list deny those skills, and the resolver
+  // applies tokens in order, so a later plain grant would overwrite the
+  // earlier deny. Additions must not override an inherited exclusion;
+  // lifting one is the explicit job of a '!name' removal entry.
+  const excluded = new Set(
+    workingBase
+      .filter((token) => token.startsWith('!'))
+      .map((token) => token.slice(1)),
+  );
+
   // Removals operate on final skill tokens: a plain name removes that
   // name, and a '!name' entry removes the exclusion token itself (lifting
   // an existing exclusion). The wildcard is never expanded here.
   const removeSet = new Set(removeList);
-  let list = [...new Set([...workingBase, ...addList])];
+  let list = [
+    ...new Set([...workingBase, ...addList.filter((a) => !excluded.has(a))]),
+  ];
   list = list.filter((skill) => !removeSet.has(skill));
 
   // A plain-name removal that the list grants implicitly via '*' must be

src/config/skills-add-remove.test.ts

@@ -191,6 +191,18 @@ describe('skills_add / skills_remove directives', () => {
     ).toEqual(['a', 'b', '!c']);
   });
 
+  test('resolveEffectiveSkills: additions do not override an inherited wildcard exclusion', () => {
+    expect(
+      resolveEffectiveSkills('oracle', ['*', '!foo'], ['foo'], undefined),
+    ).toEqual(['*', '!foo']);
+  });
+
+  test('resolveEffectiveSkills: additions skip excluded names on concrete lists too', () => {
+    expect(
+      resolveEffectiveSkills('oracle', ['a', '!b'], ['b', 'c'], undefined),
+    ).toEqual(['a', '!b', 'c']);
+  });
+
   test('getDefaultGrantedSkillNames: oracle grants in registry order', () => {
     const grants = getDefaultGrantedSkillNames('oracle');
     expect(grants[0]).toBe('simplify');
@@ -382,6 +394,20 @@ describe('skills_add / skills_remove directives', () => {
     expectNoDirectiveKeys(effective);
   });
 
+  test('loader: project skills_add does not re-grant an excluded skill', () => {
+    writeUserConfig({
+      agents: { oracle: { skills: ['*', '!foo'] } },
+    });
+    writeProjectConfig({
+      agents: { oracle: { skills_add: ['foo', 'bar'] } },
+    });
+
+    const loaded = loadPluginConfig(projectDir, { silent: true });
+    const effective = effectiveAgent(loaded);
+    expect(effective.skills).toEqual(['*', '!foo', 'bar']);
+    expectNoDirectiveKeys(effective);
+  });
+
   test('loader: plain skills entry without directives is unchanged', () => {
     writeUserConfig({
       agents: { oracle: { skills: ['*'] } },
@@ -529,4 +555,19 @@ describe('skills_add / skills_remove directives', () => {
     expect(skillPermissions?.['my-skill']).toBe('allow');
     expect(skillPermissions?.simplify).toBe('allow');
   });
+
+  test('createAgents: added skill that the base list excludes stays denied', () => {
+    const config: PluginConfig = {
+      agents: {
+        oracle: { skills: ['*', '!foo'], skills_add: ['foo'] },
+      },
+    };
+    const agents = createAgents(runtimeFor(config));
+    const oracle = agents.find((a) => a.name === 'oracle');
+    expect(oracle).toBeDefined();
+    const skillPermissions = (
+      oracle?.config.permission as Record<string, unknown>
+    )?.skill as Record<string, unknown> | undefined;
+    expect(skillPermissions?.foo).toBe('deny');
+  });
 });