From fe13d47b246f1987d1efcd3849327d02305508d1 Mon Sep 17 00:00:00 2001
From: Shirone <kacperlachowiczwp.pl@wp.pl>
Date: Wed, 7 Jan 2026 00:05:33 +0100
Subject: [PATCH] refactor: improve agent file model validation and settings
 source deduplication

- Enhanced model parsing in agent discovery to validate against allowed values and log warnings for invalid models.
- Refactored settingSources construction in AgentService to utilize Set for automatic deduplication, simplifying the merging of user and project settings with skills sources.
- Updated tests to reflect changes in allowedTools for improved functionality.

These changes enhance the robustness of agent configuration and streamline settings management.
---
 apps/server/src/lib/agent-discovery.ts        | 17 ++++++++++---
 apps/server/src/services/agent-service.ts     | 25 +++++--------------
 .../unit/providers/claude-provider.test.ts    | 12 ++++++++-
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/apps/server/src/lib/agent-discovery.ts b/apps/server/src/lib/agent-discovery.ts
index 955cbb1d..d94c87d3 100644
--- a/apps/server/src/lib/agent-discovery.ts
+++ b/apps/server/src/lib/agent-discovery.ts
@@ -68,11 +68,20 @@ async function parseAgentFile(
           .filter((t) => t && t !== '')
       : undefined;
 
-    // Parse model (optional)
+    // Parse model (optional) - validate against allowed values
     const modelMatch = frontmatter.match(/model:\s*(\w+)/);
-    const model = modelMatch
-      ? (modelMatch[1].trim() as 'sonnet' | 'opus' | 'haiku' | 'inherit')
-      : undefined;
+    const modelValue = modelMatch?.[1]?.trim();
+    const validModels = ['sonnet', 'opus', 'haiku', 'inherit'] as const;
+    const model =
+      modelValue && validModels.includes(modelValue as (typeof validModels)[number])
+        ? (modelValue as 'sonnet' | 'opus' | 'haiku' | 'inherit')
+        : undefined;
+
+    if (modelValue && !model) {
+      logger.warn(
+        `Invalid model "${modelValue}" in agent file: ${filePath}. Expected one of: ${validModels.join(', ')}`
+      );
+    }
 
     return {
       description,
diff --git a/apps/server/src/services/agent-service.ts b/apps/server/src/services/agent-service.ts
index e8678d63..1d543efc 100644
--- a/apps/server/src/services/agent-service.ts
+++ b/apps/server/src/services/agent-service.ts
@@ -289,25 +289,12 @@ export class AgentService {
       const maxTurns = sdkOptions.maxTurns;
       let allowedTools = sdkOptions.allowedTools as string[] | undefined;
 
-      // Build merged settingSources array (filter to only 'user' and 'project')
-      const settingSources: Array<'user' | 'project'> = [];
-      if (sdkOptions.settingSources) {
-        sdkOptions.settingSources.forEach((source) => {
-          if (source === 'user' || source === 'project') {
-            if (!settingSources.includes(source)) {
-              settingSources.push(source);
-            }
-          }
-        });
-      }
-      // Merge skills sources (avoid duplicates)
-      if (skillsConfig.enabled && skillsConfig.sources.length > 0) {
-        skillsConfig.sources.forEach((source) => {
-          if (!settingSources.includes(source)) {
-            settingSources.push(source);
-          }
-        });
-      }
+      // Build merged settingSources array using Set for automatic deduplication
+      const sdkSettingSources = (sdkOptions.settingSources ?? []).filter(
+        (source): source is 'user' | 'project' => source === 'user' || source === 'project'
+      );
+      const skillSettingSources = skillsConfig.enabled ? skillsConfig.sources : [];
+      const settingSources = [...new Set([...sdkSettingSources, ...skillSettingSources])];
 
       // Enhance allowedTools with Skills and Subagents tools
       if (allowedTools) {
diff --git a/apps/server/tests/unit/providers/claude-provider.test.ts b/apps/server/tests/unit/providers/claude-provider.test.ts
index 38e1bf4c..3a91652a 100644
--- a/apps/server/tests/unit/providers/claude-provider.test.ts
+++ b/apps/server/tests/unit/providers/claude-provider.test.ts
@@ -96,7 +96,17 @@ describe('claude-provider.ts', () => {
       expect(sdk.query).toHaveBeenCalledWith({
         prompt: 'Test',
         options: expect.objectContaining({
-          allowedTools: ['Read', 'Write', 'Edit', 'Glob', 'Grep', 'Bash', 'WebSearch', 'WebFetch'],
+          allowedTools: [
+            'Read',
+            'Write',
+            'Edit',
+            'Glob',
+            'Grep',
+            'Bash',
+            'WebSearch',
+            'WebFetch',
+            'Skill',
+          ],
         }),
       });
     });