diff --git a/packages/core/src/agents/agentLoader.test.ts b/packages/core/src/agents/agentLoader.test.ts index 917628f7e7..661f08d76d 100644 --- a/packages/core/src/agents/agentLoader.test.ts +++ b/packages/core/src/agents/agentLoader.test.ts @@ -242,6 +242,99 @@ Body`); /Name must be a valid slug/, ); }); + + describe('error formatting and kind inference', () => { + it('should only show local agent errors when kind is inferred as local (via kind field)', async () => { + const filePath = await writeAgentMarkdown(`--- +kind: local +name: invalid-local +# missing description +--- +Body`); + const error = await parseAgentMarkdown(filePath).catch((e) => e); + expect(error).toBeInstanceOf(AgentLoadError); + expect(error.message).toContain('Validation failed'); + expect(error.message).toContain('description: Required'); + expect(error.message).not.toContain('Remote Agent'); + }); + + it('should only show local agent errors when kind is inferred as local (via local-specific keys)', async () => { + const filePath = await writeAgentMarkdown(`--- +name: invalid-local +# missing description +tools: + - run_shell_command +--- +Body`); + const error = await parseAgentMarkdown(filePath).catch((e) => e); + expect(error).toBeInstanceOf(AgentLoadError); + expect(error.message).toContain('Validation failed'); + expect(error.message).toContain('description: Required'); + expect(error.message).not.toContain('Remote Agent'); + }); + + it('should only show remote agent errors when kind is inferred as remote (via kind field)', async () => { + const filePath = await writeAgentMarkdown(`--- +kind: remote +name: invalid-remote +# missing agent_card_url +--- +Body`); + const error = await parseAgentMarkdown(filePath).catch((e) => e); + expect(error).toBeInstanceOf(AgentLoadError); + expect(error.message).toContain('Validation failed'); + expect(error.message).toContain('agent_card_url: Required'); + expect(error.message).not.toContain('Local Agent'); + }); + + it('should only show remote agent errors when kind is inferred as remote (via remote-specific keys)', async () => { + const filePath = await writeAgentMarkdown(`--- +name: invalid-remote +auth: + type: apiKey + key: my_key +# missing agent_card_url +--- +Body`); + const error = await parseAgentMarkdown(filePath).catch((e) => e); + expect(error).toBeInstanceOf(AgentLoadError); + expect(error.message).toContain('Validation failed'); + expect(error.message).toContain('agent_card_url: Required'); + expect(error.message).not.toContain('Local Agent'); + }); + + it('should show errors for both types when kind cannot be inferred', async () => { + const filePath = await writeAgentMarkdown(`--- +name: invalid-unknown +# missing description and missing agent_card_url, no specific keys +--- +Body`); + const error = await parseAgentMarkdown(filePath).catch((e) => e); + expect(error).toBeInstanceOf(AgentLoadError); + expect(error.message).toContain('Validation failed'); + expect(error.message).toContain('(Local Agent)'); + expect(error.message).toContain('(Remote Agent)'); + expect(error.message).toContain('description: Required'); + expect(error.message).toContain('agent_card_url: Required'); + }); + + it('should format errors without a stray colon when the path is empty (e.g. strict object with unknown keys)', async () => { + const filePath = await writeAgentMarkdown(`--- +kind: local +name: my-agent +description: test +unknown_field: true +--- +Body`); + const error = await parseAgentMarkdown(filePath).catch((e) => e); + expect(error).toBeInstanceOf(AgentLoadError); + expect(error.message).toContain( + "Unrecognized key(s) in object: 'unknown_field'", + ); + expect(error.message).not.toContain(': Unrecognized key(s)'); + expect(error.message).not.toContain('Required'); + }); + }); }); describe('markdownToAgentDefinition', () => { @@ -744,5 +837,24 @@ auth: }, }); }); + + it('should throw an error for an unknown auth type in markdownToAgentDefinition', () => { + const markdown = { + kind: 'remote' as const, + name: 'unknown-auth-agent', + agent_card_url: 'https://example.com/card', + auth: { + type: 'apiKey' as const, + key: 'some-key', + }, + }; + + // Mutate the object at runtime to bypass TypeScript compile-time checks cleanly + Object.assign(markdown.auth, { type: 'some-unknown-type' }); + + expect(() => markdownToAgentDefinition(markdown)).toThrow( + /Unknown auth type: some-unknown-type/, + ); + }); }); }); diff --git a/packages/core/src/agents/agentLoader.ts b/packages/core/src/agents/agentLoader.ts index 1b9eb1ea4e..eac0985f2d 100644 --- a/packages/core/src/agents/agentLoader.ts +++ b/packages/core/src/agents/agentLoader.ts @@ -21,79 +21,6 @@ import { isValidToolName } from '../tools/tool-names.js'; import { FRONTMATTER_REGEX } from '../skills/skillLoader.js'; import { getErrorMessage } from '../utils/errors.js'; -/** - * DTO for Markdown parsing - represents the structure from frontmatter. - */ -interface FrontmatterBaseAgentDefinition { - name: string; - display_name?: string; -} - -interface FrontmatterMCPServerConfig { - command?: string; - args?: string[]; - env?: Record; - cwd?: string; - url?: string; - http_url?: string; - headers?: Record; - tcp?: string; - type?: 'sse' | 'http'; - timeout?: number; - trust?: boolean; - description?: string; - include_tools?: string[]; - exclude_tools?: string[]; -} - -interface FrontmatterLocalAgentDefinition - extends FrontmatterBaseAgentDefinition { - kind: 'local'; - description: string; - tools?: string[]; - mcp_servers?: Record; - system_prompt: string; - model?: string; - temperature?: number; - max_turns?: number; - timeout_mins?: number; -} - -/** - * Authentication configuration for remote agents in frontmatter format. - */ -interface FrontmatterAuthConfig { - type: 'apiKey' | 'http' | 'google-credentials' | 'oauth'; - // API Key - key?: string; - name?: string; - // HTTP - scheme?: string; - token?: string; - username?: string; - password?: string; - value?: string; - // Google Credentials - scopes?: string[]; - // OAuth2 - client_id?: string; - client_secret?: string; - authorization_url?: string; - token_url?: string; -} - -interface FrontmatterRemoteAgentDefinition - extends FrontmatterBaseAgentDefinition { - kind: 'remote'; - description?: string; - agent_card_url: string; - auth?: FrontmatterAuthConfig; -} - -type FrontmatterAgentDefinition = - | FrontmatterLocalAgentDefinition - | FrontmatterRemoteAgentDefinition; - /** * Error thrown when an agent definition is invalid or cannot be loaded. */ @@ -159,15 +86,13 @@ const localAgentSchema = z }) .strict(); -/** - * Base fields shared by all auth configs. - */ +type FrontmatterLocalAgentDefinition = z.infer & { + system_prompt: string; +}; + +// Base fields shared by all auth configs. const baseAuthFields = {}; -/** - * API Key auth schema. - * Supports sending key in header, query parameter, or cookie. - */ const apiKeyAuthSchema = z.object({ ...baseAuthFields, type: z.literal('apiKey'), @@ -175,11 +100,6 @@ const apiKeyAuthSchema = z.object({ name: z.string().optional(), }); -/** - * HTTP auth schema (Bearer or Basic). - * Note: Validation for scheme-specific fields is applied in authConfigSchema - * since discriminatedUnion doesn't support refined schemas directly. - */ const httpAuthSchema = z.object({ ...baseAuthFields, type: z.literal('http'), @@ -190,19 +110,12 @@ const httpAuthSchema = z.object({ value: z.string().min(1).optional(), }); -/** - * Google Credentials auth schema. - */ const googleCredentialsAuthSchema = z.object({ ...baseAuthFields, type: z.literal('google-credentials'), scopes: z.array(z.string()).optional(), }); -/** - * OAuth2 auth schema. - * authorization_url and token_url can be discovered from the agent card if omitted. - */ const oauth2AuthSchema = z.object({ ...baseAuthFields, type: z.literal('oauth'), @@ -222,18 +135,16 @@ const authConfigSchema = z ]) .superRefine((data, ctx) => { if (data.type === 'http') { - if (data.value) { - // Raw mode - only scheme and value are needed - return; - } - if (data.scheme === 'Bearer' && !data.token) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: 'Bearer scheme requires "token"', - path: ['token'], - }); - } - if (data.scheme === 'Basic') { + if (data.value) return; + if (data.scheme === 'Bearer') { + if (!data.token) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Bearer scheme requires "token"', + path: ['token'], + }); + } + } else if (data.scheme === 'Basic') { if (!data.username) { ctx.addIssue({ code: z.ZodIssueCode.custom, @@ -248,10 +159,18 @@ const authConfigSchema = z path: ['password'], }); } + } else { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `HTTP scheme "${data.scheme}" requires "value"`, + path: ['value'], + }); } } }); +type FrontmatterAuthConfig = z.infer; + const remoteAgentSchema = z .object({ kind: z.literal('remote').optional().default('remote'), @@ -263,8 +182,12 @@ const remoteAgentSchema = z }) .strict(); -// Use a Zod union to automatically discriminate between local and remote -// agent types. +type FrontmatterRemoteAgentDefinition = z.infer; + +type FrontmatterAgentDefinition = + | FrontmatterLocalAgentDefinition + | FrontmatterRemoteAgentDefinition; + const agentUnionOptions = [ { schema: localAgentSchema, label: 'Local Agent' }, { schema: remoteAgentSchema, label: 'Remote Agent' }, @@ -277,23 +200,62 @@ const markdownFrontmatterSchema = z.union([ agentUnionOptions[1].schema, ]); -function formatZodError(error: z.ZodError, context: string): string { +function guessIntendedKind(rawInput: unknown): 'local' | 'remote' | undefined { + if (typeof rawInput !== 'object' || rawInput === null) return undefined; + const input = rawInput as Partial & + Partial; + + if (input.kind === 'local') return 'local'; + if (input.kind === 'remote') return 'remote'; + + const hasLocalKeys = + 'tools' in input || + 'mcp_servers' in input || + 'model' in input || + 'temperature' in input || + 'max_turns' in input || + 'timeout_mins' in input; + const hasRemoteKeys = 'agent_card_url' in input || 'auth' in input; + + if (hasLocalKeys && !hasRemoteKeys) return 'local'; + if (hasRemoteKeys && !hasLocalKeys) return 'remote'; + + return undefined; +} + +function formatZodError( + error: z.ZodError, + context: string, + rawInput?: unknown, +): string { + const intendedKind = rawInput ? guessIntendedKind(rawInput) : undefined; + const issues = error.issues .map((i) => { - // Handle union errors specifically to give better context if (i.code === z.ZodIssueCode.invalid_union) { return i.unionErrors .map((unionError, index) => { const label = agentUnionOptions[index]?.label ?? `Agent type #${index + 1}`; + + if (intendedKind === 'local' && label === 'Remote Agent') + return null; + if (intendedKind === 'remote' && label === 'Local Agent') + return null; + const unionIssues = unionError.issues - .map((u) => `${u.path.join('.')}: ${u.message}`) + .map((u) => { + const pathStr = u.path.join('.'); + return pathStr ? `${pathStr}: ${u.message}` : u.message; + }) .join(', '); return `(${label}) ${unionIssues}`; }) + .filter(Boolean) .join('\n'); } - return `${i.path.join('.')}: ${i.message}`; + const pathStr = i.path.join('.'); + return pathStr ? `${pathStr}: ${i.message}` : i.message; }) .join('\n'); return `${context}:\n${issues}`; @@ -343,8 +305,7 @@ export async function parseAgentMarkdown( } catch (error) { throw new AgentLoadError( filePath, - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - `YAML frontmatter parsing failed: ${(error as Error).message}`, + `YAML frontmatter parsing failed: ${getErrorMessage(error)}`, ); } @@ -368,7 +329,7 @@ export async function parseAgentMarkdown( if (!result.success) { throw new AgentLoadError( filePath, - `Validation failed: ${formatZodError(result.error, 'Agent Definition')}`, + `Validation failed: ${formatZodError(result.error, 'Agent Definition', rawFrontmatter)}`, ); } @@ -383,17 +344,14 @@ export async function parseAgentMarkdown( ]; } - // Local agent validation - // Validate tools - // Construct the local agent definition - const agentDef: FrontmatterLocalAgentDefinition = { - ...frontmatter, - kind: 'local', - system_prompt: body.trim(), - }; - - return [agentDef]; + return [ + { + ...frontmatter, + kind: 'local', + system_prompt: body.trim(), + }, + ]; } /** @@ -403,15 +361,9 @@ export async function parseAgentMarkdown( function convertFrontmatterAuthToConfig( frontmatter: FrontmatterAuthConfig, ): A2AAuthConfig { - const base = {}; - switch (frontmatter.type) { case 'apiKey': - if (!frontmatter.key) { - throw new Error('Internal error: API key missing after validation.'); - } return { - ...base, type: 'apiKey', key: frontmatter.key, name: frontmatter.name, @@ -419,20 +371,13 @@ function convertFrontmatterAuthToConfig( case 'google-credentials': return { - ...base, type: 'google-credentials', scopes: frontmatter.scopes, }; - case 'http': { - if (!frontmatter.scheme) { - throw new Error( - 'Internal error: HTTP scheme missing after validation.', - ); - } + case 'http': if (frontmatter.value) { return { - ...base, type: 'http', scheme: frontmatter.scheme, value: frontmatter.value, @@ -440,40 +385,29 @@ function convertFrontmatterAuthToConfig( } switch (frontmatter.scheme) { case 'Bearer': - if (!frontmatter.token) { - throw new Error( - 'Internal error: Bearer token missing after validation.', - ); - } + // Token is required by schema validation return { - ...base, type: 'http', scheme: 'Bearer', - token: frontmatter.token, + + token: frontmatter.token!, }; case 'Basic': - if (!frontmatter.username || !frontmatter.password) { - throw new Error( - 'Internal error: Basic auth credentials missing after validation.', - ); - } + // Username/password are required by schema validation return { - ...base, type: 'http', scheme: 'Basic', - username: frontmatter.username, - password: frontmatter.password, + + username: frontmatter.username!, + + password: frontmatter.password!, }; - default: { - // Other IANA schemes without a value should not reach here after validation + default: throw new Error(`Unknown HTTP scheme: ${frontmatter.scheme}`); - } } - } case 'oauth': return { - ...base, type: 'oauth2', client_id: frontmatter.client_id, client_secret: frontmatter.client_secret, @@ -483,8 +417,12 @@ function convertFrontmatterAuthToConfig( }; default: { - const exhaustive: never = frontmatter.type; - throw new Error(`Unknown auth type: ${exhaustive}`); + const exhaustive: never = frontmatter; + const raw: unknown = exhaustive; + if (typeof raw === 'object' && raw !== null && 'type' in raw) { + throw new Error(`Unknown auth type: ${String(raw['type'])}`); + } + throw new Error('Unknown auth type'); } } } @@ -533,7 +471,7 @@ export function markdownToAgentDefinition( const modelName = markdown.model || 'inherit'; const mcpServers: Record = {}; - if (markdown.kind === 'local' && markdown.mcp_servers) { + if (markdown.mcp_servers) { for (const [name, config] of Object.entries(markdown.mcp_servers)) { mcpServers[name] = new MCPServerConfig( config.command, @@ -606,15 +544,13 @@ export async function loadAgentsFromDirectory( dirEntries = await fs.readdir(dir, { withFileTypes: true }); } catch (error) { // If directory doesn't exist, just return empty - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { return result; } result.errors.push( new AgentLoadError( dir, - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - `Could not list directory: ${(error as Error).message}`, + `Could not list directory: ${getErrorMessage(error)}`, ), ); return result; @@ -644,8 +580,7 @@ export async function loadAgentsFromDirectory( result.errors.push( new AgentLoadError( filePath, - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - `Unexpected error: ${(error as Error).message}`, + `Unexpected error: ${getErrorMessage(error)}`, ), ); }