fix(skills): inject active skill roots (#1411)

zerob13 · web-flow · commit b3644b656e68 · 2026-03-30T23:17:47.000+08:00
diff --git a/src/main/presenter/skillPresenter/index.ts b/src/main/presenter/skillPresenter/index.ts
@@ -382,13 +382,12 @@ export class SkillPresenter implements ISkillPresenter {
     const scripts = (await this.listSkillScripts(metadata.name)).filter((script) => script.enabled)
     const lines = [
       '## DeepChat Runtime Context',
-      '- Skill root: available via `SKILL_ROOT` and `DEEPCHAT_SKILL_ROOT`.',
-      '- `skill_run` executes from the current session workdir when available.',
-      '- Recommended base_directory: `<skill_root>`'
+      `- Skill root: \`${metadata.skillRoot}\`.`,
+      '- Relative paths mentioned by this skill are relative to the skill root unless stated otherwise.',
+      '- When this skill needs script execution, prefer `skill_run` over `exec`.'
     ]
 
     if (scripts.length > 0) {
-      lines.push('- Preferred execution tool: `skill_run`')
       lines.push('- Bundled runnable scripts:')
       lines.push(
         ...scripts.map((script) => {
@@ -401,7 +400,6 @@ export class SkillPresenter implements ISkillPresenter {
     }
 
     lines.push('- Do not guess script paths or change directories to locate skill files.')
-    lines.push('- Prefer `skill_run` over inline `python -c`, `node -e`, or ad-hoc shell snippets.')
 
     return lines.join('\n')
   }
diff --git a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts
@@ -664,10 +664,6 @@ export class AgentToolManager {
       return this.callProcessTool(toolName, args, conversationId)
     }
 
-    if (!this.fileSystemHandler) {
-      throw new Error('FileSystem handler not initialized')
-    }
-
     const schema = this.fileSystemSchemas[toolName as keyof typeof this.fileSystemSchemas]
     if (!schema) {
       throw new Error(`No schema found for FileSystem tool: ${toolName}`)
@@ -680,6 +676,50 @@ export class AgentToolManager {
 
     const parsedArgs = validationResult.data
 
+    if (toolName === 'exec') {
+      if (!this.bashHandler) {
+        throw new Error('Bash handler not initialized for exec tool')
+      }
+      const execArgs = parsedArgs as {
+        command: string
+        timeoutMs?: number
+        description?: string
+        cwd?: string
+        background?: boolean
+        yieldMs?: number
+      }
+      const commandResult = await this.bashHandler.executeCommand(
+        {
+          command: execArgs.command,
+          timeout: execArgs.timeoutMs,
+          description: execArgs.description ?? 'Execute command',
+          cwd: execArgs.cwd,
+          background: execArgs.background,
+          yieldMs: execArgs.yieldMs
+        },
+        {
+          conversationId
+        }
+      )
+      const content =
+        typeof commandResult.output === 'string'
+          ? commandResult.output
+          : JSON.stringify(commandResult.output)
+      return {
+        content,
+        rawData: {
+          content,
+          rtkApplied: commandResult.rtkApplied,
+          rtkMode: commandResult.rtkMode,
+          rtkFallbackReason: commandResult.rtkFallbackReason
+        }
+      }
+    }
+
+    if (!this.fileSystemHandler) {
+      throw new Error('FileSystem handler not initialized')
+    }
+
     // Get dynamic workdir from conversation settings
     let dynamicWorkdir: string | null = null
     if (conversationId) {
@@ -698,7 +738,7 @@ export class AgentToolManager {
     const baseDirectory = explicitBaseDirectory ?? dynamicWorkdir ?? undefined
     const workspaceRoot =
       dynamicWorkdir ?? this.agentWorkspacePath ?? this.getDefaultAgentWorkspacePath()
-    const allowedDirectories = this.buildAllowedDirectories(workspaceRoot, conversationId)
+    const allowedDirectories = await this.buildAllowedDirectories(workspaceRoot, conversationId)
     const fileSystemHandler = new AgentFileSystemHandler(allowedDirectories, { conversationId })
 
     try {
@@ -868,45 +908,6 @@ export class AgentToolManager {
             )
           }
         }
-        case 'exec': {
-          if (!this.bashHandler) {
-            throw new Error('Bash handler not initialized for exec tool')
-          }
-          const execArgs = parsedArgs as {
-            command: string
-            timeoutMs?: number
-            description?: string
-            cwd?: string
-            background?: boolean
-            yieldMs?: number
-          }
-          const commandResult = await this.bashHandler.executeCommand(
-            {
-              command: execArgs.command,
-              timeout: execArgs.timeoutMs,
-              description: execArgs.description ?? 'Execute command',
-              cwd: execArgs.cwd,
-              background: execArgs.background,
-              yieldMs: execArgs.yieldMs
-            },
-            {
-              conversationId
-            }
-          )
-          const content =
-            typeof commandResult.output === 'string'
-              ? commandResult.output
-              : JSON.stringify(commandResult.output)
-          return {
-            content,
-            rawData: {
-              content,
-              rtkApplied: commandResult.rtkApplied,
-              rtkMode: commandResult.rtkMode,
-              rtkFallbackReason: commandResult.rtkFallbackReason
-            }
-          }
-        }
         default:
           throw new Error(`Unknown FileSystem tool: ${toolName}`)
       }
@@ -937,7 +938,10 @@ export class AgentToolManager {
     }
   }
 
-  private buildAllowedDirectories(workspacePath: string, conversationId?: string): string[] {
+  private async buildAllowedDirectories(
+    workspacePath: string,
+    conversationId?: string
+  ): Promise<string[]> {
     const ordered: string[] = []
     const seen = new Set<string>()
     const addPath = (value?: string | null) => {
@@ -951,7 +955,14 @@ export class AgentToolManager {
 
     addPath(workspacePath)
     addPath(this.agentWorkspacePath)
-    addPath(this.configPresenter.getSkillsPath())
+
+    if (conversationId) {
+      const activeSkillRoots = await this.resolveActiveSkillRoots(conversationId)
+      for (const skillRoot of activeSkillRoots) {
+        addPath(skillRoot)
+      }
+    }
+
     addPath(path.join(app.getPath('home'), '.deepchat'))
     addPath(app.getPath('temp'))
 
@@ -965,6 +976,86 @@ export class AgentToolManager {
     return ordered
   }
 
+  private async resolveActiveSkillRoots(conversationId: string): Promise<string[]> {
+    const skillPresenter = this.getSkillPresenter()
+    if (!skillPresenter?.getActiveSkills || !skillPresenter?.getMetadataList) {
+      return []
+    }
+
+    let activeSkillNames: string[]
+    let metadataList: Awaited<ReturnType<typeof skillPresenter.getMetadataList>>
+
+    try {
+      ;[activeSkillNames, metadataList] = await Promise.all([
+        skillPresenter.getActiveSkills(conversationId),
+        skillPresenter.getMetadataList()
+      ])
+    } catch (error) {
+      logger.warn('[AgentToolManager] Failed to resolve active skill roots', {
+        conversationId,
+        error
+      })
+      return []
+    }
+
+    const metadataByName = new Map(
+      metadataList
+        .filter((metadata) => metadata?.name?.trim())
+        .map((metadata) => [metadata.name.trim(), metadata])
+    )
+    const roots: string[] = []
+
+    for (const skillName of activeSkillNames) {
+      const normalizedSkillName = skillName?.trim()
+      if (!normalizedSkillName) {
+        continue
+      }
+
+      const metadata = metadataByName.get(normalizedSkillName)
+      if (!metadata) {
+        logger.warn(
+          '[AgentToolManager] Active skill metadata missing during file allowlist build',
+          {
+            conversationId,
+            skillName: normalizedSkillName
+          }
+        )
+        continue
+      }
+
+      const skillRoot = metadata.skillRoot?.trim()
+      if (!skillRoot) {
+        logger.warn('[AgentToolManager] Active skill root missing during file allowlist build', {
+          conversationId,
+          skillName: normalizedSkillName
+        })
+        continue
+      }
+
+      try {
+        const resolvedRoot = path.resolve(skillRoot)
+        if (!fs.existsSync(resolvedRoot) || !fs.statSync(resolvedRoot).isDirectory()) {
+          logger.warn('[AgentToolManager] Active skill root is not a directory', {
+            conversationId,
+            skillName: normalizedSkillName,
+            skillRoot: resolvedRoot
+          })
+          continue
+        }
+        roots.push(resolvedRoot)
+      } catch (error) {
+        logger.warn('[AgentToolManager] Failed to normalize active skill root', {
+          conversationId,
+          skillName: normalizedSkillName,
+          skillRoot,
+          error
+        })
+      }
+    }
+
+    return roots
+  }
+
   private async resolveValidatedReadPath(
     fileSystemHandler: AgentFileSystemHandler,
     requestedPath: string,
@@ -1470,8 +1561,13 @@ export class AgentToolManager {
 
       const workspaceRoot =
         dynamicWorkdir ?? this.agentWorkspacePath ?? this.getDefaultAgentWorkspacePath()
-      const allowedDirectories = this.buildAllowedDirectories(workspaceRoot, conversationId)
+      const allowedDirectories = await this.buildAllowedDirectories(workspaceRoot, conversationId)
       const fileSystemHandler = new AgentFileSystemHandler(allowedDirectories, { conversationId })
+      const explicitBaseDirectory =
+        typeof args.base_directory === 'string' && args.base_directory.trim().length > 0
+          ? args.base_directory
+          : undefined
+      const baseDirectory = explicitBaseDirectory ?? dynamicWorkdir ?? undefined
 
       // Collect target paths
       const targets = this.collectWriteTargets(toolName, args)
@@ -1485,7 +1581,7 @@ export class AgentToolManager {
       // Check each path
       const denied: string[] = []
       for (const target of targets) {
-        const resolved = fileSystemHandler.resolvePath(target, undefined)
+        const resolved = fileSystemHandler.resolvePath(target, baseDirectory)
         if (!fileSystemHandler.isPathAllowedAbsolute(resolved)) {
           denied.push(target)
         }
diff --git a/test/main/presenter/skillPresenter/skillPresenter.test.ts b/test/main/presenter/skillPresenter/skillPresenter.test.ts
@@ -358,9 +358,14 @@ describe('SkillPresenter', () => {
       expect(content).toBeTruthy()
       expect(content?.name).toBe('test-skill')
       expect(content?.content).toContain('Skill content')
-      expect(content?.content).toContain('Skill root: resolved server-side by `skill_run`.')
-      expect(content?.content).toContain('Recommended base_directory: `<skill_root>`')
-      expect(content?.content).not.toContain('/mock/home/.deepchat/skills/test-skill')
+      expect(content?.content).toContain('Skill root: `')
+      expect(content?.content).toContain('/.deepchat/skills/test-skill`.')
+      expect(content?.content).toContain(
+        'Relative paths mentioned by this skill are relative to the skill root unless stated otherwise.'
+      )
+      expect(content?.content).toContain(
+        'When this skill needs script execution, prefer `skill_run` over `exec`.'
+      )
     })
 
     it('should return null for non-existent skill', async () => {
diff --git a/test/main/presenter/toolPresenter/agentTools/agentToolManagerSkillAccess.test.ts b/test/main/presenter/toolPresenter/agentTools/agentToolManagerSkillAccess.test.ts
