Actually validate subagents, locally + database

jahooma · jahooma · commit dd8b571c5f07 · 2025-08-30T13:46:17.000-07:00
diff --git a/backend/src/templates/agent-registry.ts b/backend/src/templates/agent-registry.ts
@@ -80,7 +80,6 @@ async function fetchAgentFromDatabase(parsedAgentId: {
       { ...rawAgentData, id: agentId },
       {
         filePath: `${publisherId}/${agentId}@${agentConfig.version}`,
-        skipSubagentValidation: true,
       },
     )
 
diff --git a/common/src/templates/agent-validation.ts b/common/src/templates/agent-validation.ts
@@ -1,9 +1,6 @@
 import { convertJsonSchemaToZod } from 'zod-from-json-schema'
 
-import {
-  formatSpawnableAgentError,
-  validateSpawnableAgents,
-} from '../util/agent-template-validation'
+import { validateSpawnableAgents } from '../util/agent-template-validation'
 import { logger } from '../util/logger'
 import {
   DynamicAgentDefinitionSchema,
@@ -22,8 +19,9 @@ export interface DynamicAgentValidationError {
  */
 export function collectAgentIds(
   agentTemplates: Record<string, DynamicAgentTemplate> = {},
-): string[] {
+): { agentIds: string[]; spawnableAgentIds: string[] } {
   const agentIds: string[] = []
+  const spawnableAgentIds: string[] = []
   const jsonFiles = Object.keys(agentTemplates)
 
   for (const filePath of jsonFiles) {
@@ -37,6 +35,9 @@ export function collectAgentIds(
       if (content.id && typeof content.id === 'string') {
         agentIds.push(content.id)
       }
+      if (Array.isArray(content.spawnableAgents)) {
+        spawnableAgentIds.push(...content.spawnableAgents)
+      }
     } catch (error) {
       // Log but don't fail the collection process for other errors
       logger.debug(
@@ -46,7 +47,29 @@ export function collectAgentIds(
     }
   }
 
-  return agentIds
+  return { agentIds, spawnableAgentIds }
+}
+
+export async function validateAgentsWithSpawnableAgents(
+  agentTemplates: Record<string, any> = {},
+): Promise<{
+  templates: Record<string, AgentTemplate>
+  dynamicTemplates: Record<string, DynamicAgentTemplate>
+  validationErrors: DynamicAgentValidationError[]
+}> {
+  const { agentIds, spawnableAgentIds } = collectAgentIds(agentTemplates)
+  const { validationErrors } = await validateSpawnableAgents(
+    spawnableAgentIds,
+    agentIds,
+  )
+  if (validationErrors.length > 0) {
+    return {
+      templates: {},
+      dynamicTemplates: {},
+      validationErrors,
+    }
+  }
+  return validateAgents(agentTemplates)
 }
 
 /**
@@ -73,10 +96,7 @@ export function validateAgents(agentTemplates: Record<string, any> = {}): {
 
   const agentKeys = Object.keys(agentTemplates)
 
-  // Pass 1: Collect all agent IDs from template files
-  const dynamicAgentIds = collectAgentIds(agentTemplates)
-
-  // Pass 2: Load and validate each agent template
+  // Load and validate each agent template
   for (const agentKey of agentKeys) {
     const content = agentTemplates[agentKey]
     try {
@@ -86,7 +106,6 @@ export function validateAgents(agentTemplates: Record<string, any> = {}): {
 
       const validationResult = validateSingleAgent(content, {
         filePath: agentKey,
-        dynamicAgentIds,
       })
 
       if (!validationResult.success) {
@@ -154,21 +173,15 @@ export function validateAgents(agentTemplates: Record<string, any> = {}): {
 export function validateSingleAgent(
   template: any,
   options?: {
-    dynamicAgentIds?: string[]
     filePath?: string
-    skipSubagentValidation?: boolean
   },
 ): {
   success: boolean
   agentTemplate?: AgentTemplate
   dynamicAgentTemplate?: DynamicAgentTemplate
   error?: string
 } {
-  const {
-    filePath,
-    skipSubagentValidation = true,
-    dynamicAgentIds = [],
-  } = options || {}
+  const { filePath } = options || {}
 
   try {
     // First validate against the Zod schema
@@ -203,23 +216,6 @@ export function validateSingleAgent(
       }
     }
 
-    // Validate spawnable agents (skip if requested, e.g., for database agents)
-    if (!skipSubagentValidation) {
-      const spawnableAgentValidation = validateSpawnableAgents(
-        validatedConfig.spawnableAgents,
-        dynamicAgentIds,
-      )
-      if (!spawnableAgentValidation.valid) {
-        return {
-          success: false,
-          error: formatSpawnableAgentError(
-            spawnableAgentValidation.invalidAgents,
-            spawnableAgentValidation.availableAgents,
-          ),
-        }
-      }
-    }
-
     // Convert schemas and handle validation errors
     let inputSchema: AgentTemplate['inputSchema']
     try {
diff --git a/common/src/templates/fetch-agent.ts b/common/src/templates/fetch-agent.ts
@@ -0,0 +1,24 @@
+import { db } from '@codebuff/common/db'
+import * as schema from '@codebuff/common/db/schema'
+import { and, eq } from 'drizzle-orm'
+
+export async function fetchAgent(
+  agentId: string,
+  version: string,
+  publisher: string,
+) {
+  // Find the agent template
+  const agent = await db
+    .select()
+    .from(schema.agentConfig)
+    .where(
+      and(
+        eq(schema.agentConfig.id, agentId),
+        eq(schema.agentConfig.version, version),
+        eq(schema.agentConfig.publisher_id, publisher),
+      ),
+    )
+    .then((rows) => rows[0])
+
+  return agent
+}
diff --git a/common/src/util/agent-id-parsing.ts b/common/src/util/agent-id-parsing.ts
@@ -8,6 +8,7 @@ export function parseAgentId(fullAgentId: string): {
   publisherId?: string
   agentId?: string
   version?: string
+  givenAgentId: string
 } {
   // Check if it's in the publisher/agent-id[@version] format
   const parts = fullAgentId.split('/')
@@ -17,40 +18,65 @@ export function parseAgentId(fullAgentId: string): {
     const [publisherId, agentNameWithVersion] = parts
 
     if (!publisherId || !agentNameWithVersion) {
-      return { publisherId: undefined, agentId: undefined, version: undefined }
+      return {
+        publisherId: undefined,
+        agentId: undefined,
+        version: undefined,
+        givenAgentId: fullAgentId,
+      }
     }
 
     // Check for version suffix
     const versionMatch = agentNameWithVersion.match(/^(.+)@(.+)$/)
     if (versionMatch) {
       const [, agentId, version] = versionMatch
-      return { publisherId, agentId, version }
+      return { publisherId, agentId, version, givenAgentId: fullAgentId }
     }
 
-    return { publisherId, agentId: agentNameWithVersion }
+    return {
+      publisherId,
+      agentId: agentNameWithVersion,
+      givenAgentId: fullAgentId,
+    }
   } else if (parts.length === 1) {
     // Just agent name (for backward compatibility)
     const agentNameWithVersion = parts[0]
 
     if (!agentNameWithVersion) {
-      return { publisherId: undefined, agentId: undefined, version: undefined }
+      return {
+        publisherId: undefined,
+        agentId: undefined,
+        version: undefined,
+        givenAgentId: fullAgentId,
+      }
     }
 
     // Check for version suffix
     const versionMatch = agentNameWithVersion.match(/^(.+)@(.+)$/)
     if (versionMatch) {
       const [, agentId, version] = versionMatch
-      return { publisherId: undefined, agentId, version }
+      return {
+        publisherId: undefined,
+        agentId,
+        version,
+        givenAgentId: fullAgentId,
+      }
     }
 
     return {
       publisherId: undefined,
       agentId: agentNameWithVersion,
       version: undefined,
+      givenAgentId: fullAgentId,
     }
   }
 
-  return { publisherId: undefined, agentId: undefined, version: undefined }
+  return {
+    publisherId: undefined,
+    agentId: undefined,
+    version: undefined,
+    givenAgentId: fullAgentId,
+  }
 }
 
 /**
diff --git a/common/src/util/agent-template-validation.ts b/common/src/util/agent-template-validation.ts
@@ -1,4 +1,7 @@
+import { DynamicAgentValidationError } from 'src/templates/agent-validation'
+import { fetchAgent } from '../templates/fetch-agent'
 import { AgentTemplateTypes } from '../types/session-state'
+import { parseAgentId } from './agent-id-parsing'
 
 export interface SubagentValidationResult {
   valid: boolean
@@ -9,24 +12,56 @@ export interface SubagentValidationResult {
  * Centralized validation for spawnable agents.
  * Validates that all spawnable agents reference valid agent types.
  */
-export function validateSpawnableAgents(
+export async function validateSpawnableAgents(
   spawnableAgents: string[],
   dynamicAgentIds: string[],
-): SubagentValidationResult & { availableAgents: string[] } {
+): Promise<
+  SubagentValidationResult & {
+    availableAgents: string[]
+    validationErrors: DynamicAgentValidationError[]
+  }
+> {
   // Build complete list of available agent types (normalized)
   const availableAgentTypes = [
     ...Object.values(AgentTemplateTypes),
     ...dynamicAgentIds,
   ]
+  const parsedIds = spawnableAgents.map((id) => parseAgentId(id))
+  const invalidIds: string[] = []
+  const validationErrors: DynamicAgentValidationError[] = []
+  for (const id of parsedIds) {
+    const { publisherId, agentId, version, givenAgentId } = id
+
+    if (availableAgentTypes.includes(givenAgentId)) {
+      // Agent provided by dynamic definitions.
+      continue
+    }
+    if (!publisherId || !agentId || !version) {
+      invalidIds.push(givenAgentId)
+      validationErrors.push({
+        filePath: givenAgentId,
+        message: `Invalid agent ID: ${givenAgentId}. Not found. You must include the publisher, agent id, and version if the agent is not defined locally.`,
+      })
+      continue
+    }
 
-  // Find invalid agents (those not in available types after normalization)
-  const invalidAgents = spawnableAgents.filter(
-    (agent, index) => !availableAgentTypes.includes(spawnableAgents[index]),
-  )
+    // Check if agent exists in database.
+    const agent = await fetchAgent(agentId, version, publisherId)
+    if (!agent) {
+      invalidIds.push(givenAgentId)
+      validationErrors.push({
+        filePath: givenAgentId,
+        message: `Invalid agent ID: ${givenAgentId}. Agent not found in database.`,
+      })
+      continue
+    }
+    availableAgentTypes.push(givenAgentId)
+  }
 
   return {
-    valid: invalidAgents.length === 0,
-    invalidAgents,
+    valid: validationErrors.length === 0,
+    invalidAgents: invalidIds,
+    validationErrors,
     availableAgents: availableAgentTypes,
   }
 }
diff --git a/web/src/app/api/agents/publish/route.ts b/web/src/app/api/agents/publish/route.ts
@@ -1,6 +1,6 @@
 import db from '@codebuff/common/db'
 import * as schema from '@codebuff/common/db/schema'
-import { validateAgents } from '@codebuff/common/templates/agent-validation'
+import { validateAgentsWithSpawnableAgents } from '@codebuff/common/templates/agent-validation'
 import { publishAgentsRequestSchema } from '@codebuff/common/types/api/agents/publish'
 import {
   checkAuthToken,
@@ -70,7 +70,8 @@ export async function POST(request: NextRequest) {
       {} as Record<string, any>
     )
 
-    const { validationErrors, dynamicTemplates } = validateAgents(agentMap)
+    const { validationErrors, dynamicTemplates } =
+      await validateAgentsWithSpawnableAgents(agentMap)
     const agents = Object.values(dynamicTemplates)
 
     if (validationErrors.length > 0) {
@@ -132,7 +133,6 @@ export async function POST(request: NextRequest) {
 
     const requestedPublisherId = publisherIds[0]!
 
-
     // Verify user has access to the requested publisher
     const publisherResult = await db
       .select({
diff --git a/web/src/app/api/agents/validate/route.ts b/web/src/app/api/agents/validate/route.ts
@@ -1,4 +1,4 @@
-import { validateAgents } from '@codebuff/common/templates/agent-validation'
+import { validateAgentsWithSpawnableAgents } from '@codebuff/common/templates/agent-validation'
 import { NextResponse } from 'next/server'
 
 import { logger } from '@/util/logger'
@@ -33,7 +33,7 @@ export async function POST(request: NextRequest): Promise<NextResponse> {
       agentDefinitions.map((config) => [config.id, config])
     )
     const { templates: configs, validationErrors } =
-      validateAgents(definitionsObject)
+      await validateAgentsWithSpawnableAgents(definitionsObject)
 
     if (validationErrors.length > 0) {
       logger.warn(

Original file line number	Diff line number	Diff line change
`@@ -80,7 +80,6 @@ async function fetchAgentFromDatabase(parsedAgentId: {`
`80`	`80`	`{ ...rawAgentData, id: agentId },`
`81`	`81`	`{`
`82`	`82`	filePath: `${publisherId}/${agentId}@${agentConfig.version}`,
`83`		`- skipSubagentValidation: true,`
`84`	`83`	`},`
`85`	`84`	`)`
`86`	`85`