From 0cfe30450687342c1a84047023954ff47d466e64 Mon Sep 17 00:00:00 2001 From: Adam Weidman Date: Tue, 24 Mar 2026 11:31:39 -0400 Subject: [PATCH 1/3] refactor(core): use inferred types and discriminated unions for agent loader --- packages/core/src/agents/agentLoader.ts | 256 +++++++++--------------- 1 file changed, 92 insertions(+), 164 deletions(-) diff --git a/packages/core/src/agents/agentLoader.ts b/packages/core/src/agents/agentLoader.ts index 2cb7b3c439..00c4231e7e 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' | 'oauth2'; - // 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('oauth2'), @@ -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,20 +200,55 @@ 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}`) .join(', '); return `(${label}) ${unionIssues}`; }) + .filter(Boolean) .join('\n'); } return `${i.path.join('.')}: ${i.message}`; @@ -343,8 +301,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 +325,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 +340,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 +357,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 +367,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 +381,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 'oauth2': return { - ...base, type: 'oauth2', client_id: frontmatter.client_id, client_secret: frontmatter.client_secret, @@ -483,8 +413,9 @@ function convertFrontmatterAuthToConfig( }; default: { - const exhaustive: never = frontmatter.type; - throw new Error(`Unknown auth type: ${exhaustive}`); + const exhaustive: never = frontmatter; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion, @typescript-eslint/no-explicit-any + throw new Error(`Unknown auth type: ${(exhaustive as any).type}`); } } } @@ -533,7 +464,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 +537,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 +573,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)}`, ), ); } From 236249604e738909fb68b52dd95366932682768f Mon Sep 17 00:00:00 2001 From: Adam Weidman Date: Tue, 24 Mar 2026 11:54:47 -0400 Subject: [PATCH 2/3] docs(core): update remote agents documentation for oauth --- docs/core/remote-agents.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/core/remote-agents.md b/docs/core/remote-agents.md index 2e34a9dbc4..05975421fe 100644 --- a/docs/core/remote-agents.md +++ b/docs/core/remote-agents.md @@ -104,7 +104,7 @@ Gemini CLI supports the following authentication types: | `apiKey` | Send a static API key as an HTTP header. | | `http` | HTTP authentication (Bearer token, Basic credentials, or any IANA-registered scheme). | | `google-credentials` | Google Application Default Credentials (ADC). Automatically selects access or identity tokens. | -| `oauth2` | OAuth 2.0 Authorization Code flow with PKCE. Opens a browser for interactive sign-in. | +| `oauth` | OAuth 2.0 Authorization Code flow with PKCE. Opens a browser for interactive sign-in. | ### Dynamic values @@ -263,7 +263,7 @@ hosts: Requests to any other host will be rejected with an error. If your agent is hosted on a different domain, use one of the other auth types (`apiKey`, `http`, -or `oauth2`). +or `oauth`). #### Examples @@ -297,7 +297,7 @@ auth: --- ``` -### OAuth 2.0 (`oauth2`) +### OAuth 2.0 (`oauth`) Performs an interactive OAuth 2.0 Authorization Code flow with PKCE. On first use, Gemini CLI opens your browser for sign-in and persists the resulting tokens @@ -305,7 +305,7 @@ for subsequent requests. | Field | Type | Required | Description | | :------------------ | :------- | :------- | :------------------------------------------------------------------------------------------------------------------------------------------------- | -| `type` | string | Yes | Must be `oauth2`. | +| `type` | string | Yes | Must be `oauth`. | | `client_id` | string | Yes\* | OAuth client ID. Required for interactive auth. | | `client_secret` | string | No\* | OAuth client secret. Required by most authorization servers (confidential clients). Can be omitted for public clients that don't require a secret. | | `scopes` | string[] | No | Requested scopes. Can also be discovered from the agent card. | @@ -318,7 +318,7 @@ kind: remote name: oauth-agent agent_card_url: https://example.com/.well-known/agent.json auth: - type: oauth2 + type: oauth client_id: my-client-id.apps.example.com --- ``` From e48a058d633c7e2154d0f95423db32021e2a2dda Mon Sep 17 00:00:00 2001 From: Adam Weidman Date: Tue, 24 Mar 2026 13:20:41 -0400 Subject: [PATCH 3/3] test(core): add comprehensive UX tests for agent loading error formatting --- packages/core/src/agents/agentLoader.test.ts | 93 ++++++++++++++++++++ packages/core/src/agents/agentLoader.ts | 8 +- 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/packages/core/src/agents/agentLoader.test.ts b/packages/core/src/agents/agentLoader.test.ts index ea7ef0b2c3..1ab073fb59 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', () => { diff --git a/packages/core/src/agents/agentLoader.ts b/packages/core/src/agents/agentLoader.ts index 00c4231e7e..cf85674496 100644 --- a/packages/core/src/agents/agentLoader.ts +++ b/packages/core/src/agents/agentLoader.ts @@ -244,14 +244,18 @@ function formatZodError( 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}`;