From 7f7cc37d47aa6c17b2bf05f5d5d5d9b6a63da8be Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 26 May 2026 18:34:12 -0400 Subject: [PATCH 1/4] feat: instrument config command --- integ-tests/config.test.ts | 101 +++++++++++++++++++++++++ src/cli/cli.ts | 2 + src/cli/commands/config/actions.ts | 71 +++++++++++++++++ src/cli/commands/config/command.ts | 31 ++++++++ src/cli/commands/config/index.ts | 1 + src/cli/commands/config/types.ts | 3 + src/cli/tui/copy.ts | 1 + src/lib/schemas/io/global-config.ts | 33 ++++---- src/lib/utils/__tests__/object.test.ts | 46 +++++++++++ src/lib/utils/__tests__/zod.test.ts | 51 ++++++++++++- src/lib/utils/index.ts | 2 +- src/lib/utils/object.ts | 15 ++++ src/lib/utils/zod.ts | 20 +++++ 13 files changed, 362 insertions(+), 15 deletions(-) create mode 100644 integ-tests/config.test.ts create mode 100644 src/cli/commands/config/actions.ts create mode 100644 src/cli/commands/config/command.ts create mode 100644 src/cli/commands/config/index.ts create mode 100644 src/cli/commands/config/types.ts create mode 100644 src/lib/utils/__tests__/object.test.ts create mode 100644 src/lib/utils/object.ts diff --git a/integ-tests/config.test.ts b/integ-tests/config.test.ts new file mode 100644 index 000000000..c9620f845 --- /dev/null +++ b/integ-tests/config.test.ts @@ -0,0 +1,101 @@ +import { spawnAndCollect } from '../src/test-utils/cli-runner.js'; +import { mkdtempSync, readFileSync } from 'node:fs'; +import { rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, describe, expect, it } from 'vitest'; + +const testConfigDir = mkdtempSync(join(tmpdir(), 'agentcore-config-integ-')); +const cliPath = join(__dirname, '..', 'dist', 'cli', 'index.mjs'); + +function run(args: string[]) { + return spawnAndCollect('node', [cliPath, ...args], tmpdir(), { + AGENTCORE_SKIP_INSTALL: '1', + AGENTCORE_CONFIG_DIR: testConfigDir, + }); +} + +function readConfig() { + return JSON.parse(readFileSync(join(testConfigDir, 'config.json'), 'utf-8')); +} + +describe('config command', () => { + afterAll(() => rm(testConfigDir, { recursive: true, force: true })); + + it('lists config with only installationId when fresh', async () => { + const result = await run(['config']); + expect(result.exitCode).toBe(0); + const parsed = JSON.parse(result.stdout); + expect(parsed.installationId).toBeDefined(); + }); + + it('sets a string value', async () => { + const result = await run(['config', 'uvIndex', 'https://example.com']); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Set uvIndex = https://example.com'); + expect(readConfig().uvIndex).toBe('https://example.com'); + }); + + it('gets a value', async () => { + const result = await run(['config', 'uvIndex']); + expect(result.exitCode).toBe(0); + expect(result.stdout.trim()).toBe('https://example.com'); + }); + + it('sets a nested value with dot notation', async () => { + const result = await run(['config', 'telemetry.endpoint', 'https://metrics.example.com']); + expect(result.exitCode).toBe(0); + expect(readConfig().telemetry.endpoint).toBe('https://metrics.example.com'); + }); + + it('gets a nested value with dot notation', async () => { + const result = await run(['config', 'telemetry.endpoint']); + expect(result.exitCode).toBe(0); + expect(result.stdout.trim()).toBe('https://metrics.example.com'); + }); + + it('gets an object value as JSON', async () => { + const result = await run(['config', 'telemetry']); + expect(result.exitCode).toBe(0); + const parsed = JSON.parse(result.stdout); + expect(parsed.endpoint).toBe('https://metrics.example.com'); + }); + + it('sets a boolean value via JSON parsing', async () => { + const result = await run(['config', 'telemetry.enabled', 'true']); + expect(result.exitCode).toBe(0); + expect(readConfig().telemetry.enabled).toBe(true); + }); + + it('sets a numeric value via JSON parsing', async () => { + const result = await run(['config', 'transactionSearchIndexPercentage', '50']); + expect(result.exitCode).toBe(0); + expect(readConfig().transactionSearchIndexPercentage).toBe(50); + }); + + it('rejects invalid value for a typed key', async () => { + const result = await run(['config', 'telemetry.enabled', 'notabool']); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('Invalid value'); + }); + + it('rejects unknown keys', async () => { + const result = await run(['config', 'foo.bar.baz', 'hello']); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('Invalid value'); + }); + + it('returns error for unset key', async () => { + const result = await run(['config', 'disableTransactionSearch']); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('is not set'); + }); + + it('lists all config after mutations', async () => { + const result = await run(['config']); + expect(result.exitCode).toBe(0); + const parsed = JSON.parse(result.stdout); + expect(parsed.uvIndex).toBe('https://example.com'); + expect(parsed.telemetry.endpoint).toBe('https://metrics.example.com'); + }); +}); diff --git a/src/cli/cli.ts b/src/cli/cli.ts index db18ebd83..a24ef4e53 100644 --- a/src/cli/cli.ts +++ b/src/cli/cli.ts @@ -3,6 +3,7 @@ import { registerABTestCommand } from './commands/abtest'; import { registerAdd } from './commands/add'; import { registerAddTool } from './commands/add/tool-command'; import { registerArchive } from './commands/archive'; +import { registerConfig } from './commands/config'; import { registerConfigBundle } from './commands/config-bundle'; import { registerCreate } from './commands/create'; import { registerDataset } from './commands/dataset'; @@ -205,6 +206,7 @@ export function registerCommands(program: Command) { registerUpdate(program); registerValidate(program); registerConfigBundle(program); + registerConfig(program); registerDataset(program); registerArchive(program); diff --git a/src/cli/commands/config/actions.ts b/src/cli/commands/config/actions.ts new file mode 100644 index 000000000..0954a81e9 --- /dev/null +++ b/src/cli/commands/config/actions.ts @@ -0,0 +1,71 @@ +import { readGlobalConfig, updateGlobalConfig, validateGlobalConfig } from '../../../lib/schemas/io/global-config.js'; +import { deepMerge } from '../../../lib/utils/object.js'; +import type { ConfigResult } from './types.js'; + +export async function handleConfigList(): Promise { + const config = await readGlobalConfig(); + return { success: true, message: JSON.stringify(config, null, 2) }; +} + +export async function handleConfigGet(key: string): Promise { + const config = await readGlobalConfig(); + const value = getByPath(config, key); + if (value === undefined) { + return { success: false, error: new Error(`Key "${key}" is not set.`) }; + } + const message = JSON.stringify(value, null, 2); + return { success: true, message }; +} + +export async function handleConfigSet(key: string, raw: string): Promise { + const value = parseValue(raw); + const existing = await readGlobalConfig(); + const partial = buildNestedObject(key, value); + const merged = deepMerge(existing, partial); + + const validation = validateGlobalConfig(merged); + if (!validation.success) { + return { success: false, error: new Error(`Invalid value "${raw}" for key "${key}".`) }; + } + + const ok = await updateGlobalConfig(partial); + if (!ok) { + return { success: false, error: new Error(`Could not write config.`) }; + } + return { success: true, message: `Set ${key} = ${raw}` }; +} + +function parseValue(raw: string): unknown { + try { + return JSON.parse(raw); + } catch { + return raw; + } +} + +function isRecord(val: unknown): val is Record { + return typeof val === 'object' && val !== null && !Array.isArray(val); +} + +function getByPath(obj: Record, path: string): unknown { + let current: unknown = obj; + for (const part of path.split('.')) { + if (!isRecord(current)) return undefined; + current = current[part]; + } + return current; +} + +function buildNestedObject(path: string, value: unknown): Record { + const parts = path.split('.'); + const leaf = parts.pop(); + if (!leaf) return {}; + const result: Record = {}; + const inner = parts.reduce>((acc, part) => { + const next: Record = {}; + acc[part] = next; + return next; + }, result); + inner[leaf] = value; + return result; +} diff --git a/src/cli/commands/config/command.ts b/src/cli/commands/config/command.ts new file mode 100644 index 000000000..c61e7fae6 --- /dev/null +++ b/src/cli/commands/config/command.ts @@ -0,0 +1,31 @@ +import { COMMAND_DESCRIPTIONS } from '../../tui/copy.js'; +import { handleConfigGet, handleConfigList, handleConfigSet } from './actions.js'; +import type { ConfigResult } from './types.js'; +import type { Command } from '@commander-js/extra-typings'; + +function resolveAction(key?: string, value?: string): () => Promise { + if (!key) return () => handleConfigList(); + if (value === undefined) return () => handleConfigGet(key); + return () => handleConfigSet(key, value); +} + +function printResult(result: ConfigResult): void { + if (result.success) { + console.log(result.message); + } else { + console.error(result.error.message); + } +} + +export function registerConfig(program: Command) { + program + .command('config') + .description(COMMAND_DESCRIPTIONS.config) + .argument('[key]', 'Config key in dot notation (e.g. telemetry.enabled)') + .argument('[value]', 'Value to set') + .action(async (key?: string, value?: string) => { + const result = await resolveAction(key, value)(); + printResult(result); + if (!result.success) process.exit(1); + }); +} diff --git a/src/cli/commands/config/index.ts b/src/cli/commands/config/index.ts new file mode 100644 index 000000000..62e626078 --- /dev/null +++ b/src/cli/commands/config/index.ts @@ -0,0 +1 @@ +export { registerConfig } from './command.js'; diff --git a/src/cli/commands/config/types.ts b/src/cli/commands/config/types.ts new file mode 100644 index 000000000..1ff9d69e8 --- /dev/null +++ b/src/cli/commands/config/types.ts @@ -0,0 +1,3 @@ +import type { Result } from '../../../lib/result.js'; + +export type ConfigResult = Result<{ message: string }>; diff --git a/src/cli/tui/copy.ts b/src/cli/tui/copy.ts index 59b574a76..def2450b3 100644 --- a/src/cli/tui/copy.ts +++ b/src/cli/tui/copy.ts @@ -56,6 +56,7 @@ export const COMMAND_DESCRIPTIONS = { validate: 'Validate agentcore/ config files.', 'config-bundle': '[preview] Manage configuration bundle versions and diffs.', archive: '[preview] Archive (delete) a batch evaluation or recommendation on the service and clear local history.', + config: 'Adjust global configuration settings such as telemetry opt-out status', } as const; /** diff --git a/src/lib/schemas/io/global-config.ts b/src/lib/schemas/io/global-config.ts index fc64eb39b..aecced026 100644 --- a/src/lib/schemas/io/global-config.ts +++ b/src/lib/schemas/io/global-config.ts @@ -1,3 +1,4 @@ +import { withCatchAll } from '../../utils/zod.js'; import { readFileSync } from 'fs'; import { mkdir, readFile, writeFile } from 'fs/promises'; import { randomUUID } from 'node:crypto'; @@ -8,25 +9,31 @@ import { z } from 'zod'; export const GLOBAL_CONFIG_DIR = process.env.AGENTCORE_CONFIG_DIR ?? join(homedir(), '.agentcore'); export const GLOBAL_CONFIG_FILE = join(GLOBAL_CONFIG_DIR, 'config.json'); -const GlobalConfigSchema = z +const GlobalConfigSchemaStrict = z .object({ - installationId: z.string().optional().catch(undefined), - uvDefaultIndex: z.string().optional().catch(undefined), - uvIndex: z.string().optional().catch(undefined), - disableTransactionSearch: z.boolean().optional().catch(undefined), - transactionSearchIndexPercentage: z.number().int().min(0).max(100).optional().catch(undefined), + installationId: z.string().optional(), + uvDefaultIndex: z.string().optional(), + uvIndex: z.string().optional(), + disableTransactionSearch: z.boolean().optional(), + transactionSearchIndexPercentage: z.number().int().min(0).max(100).optional(), telemetry: z .object({ - enabled: z.boolean().optional().catch(undefined), - endpoint: z.string().optional().catch(undefined), - audit: z.boolean().optional().catch(undefined), + enabled: z.boolean().optional(), + endpoint: z.string().optional(), + audit: z.boolean().optional(), }) - .optional() - .catch(undefined), + .strict() + .optional(), }) - .passthrough(); + .strict(); -export type GlobalConfig = z.infer; +const GlobalConfigSchema = withCatchAll(GlobalConfigSchemaStrict); + +export type GlobalConfig = z.infer; + +export function validateGlobalConfig(data: unknown): { success: boolean; error?: z.ZodError } { + return GlobalConfigSchemaStrict.safeParse(data); +} export async function readGlobalConfig(configFile = GLOBAL_CONFIG_FILE): Promise { try { diff --git a/src/lib/utils/__tests__/object.test.ts b/src/lib/utils/__tests__/object.test.ts new file mode 100644 index 000000000..1bccca8e6 --- /dev/null +++ b/src/lib/utils/__tests__/object.test.ts @@ -0,0 +1,46 @@ +import { deepMerge } from '../object.js'; +import { describe, expect, it } from 'vitest'; + +describe('deepMerge', () => { + it('merges flat objects', () => { + expect(deepMerge({ a: 1 }, { b: 2 })).toEqual({ a: 1, b: 2 }); + }); + + it('overwrites primitive values', () => { + expect(deepMerge({ a: 1 }, { a: 2 })).toEqual({ a: 2 }); + }); + + it('recursively merges nested objects', () => { + const target = { nested: { a: 1, b: 2 } }; + const source = { nested: { b: 3, c: 4 } }; + expect(deepMerge(target, source)).toEqual({ nested: { a: 1, b: 3, c: 4 } }); + }); + + it('overwrites non-object with object', () => { + expect(deepMerge({ a: 'string' }, { a: { nested: true } })).toEqual({ a: { nested: true } }); + }); + + it('overwrites object with non-object', () => { + expect(deepMerge({ a: { nested: true } }, { a: 'string' })).toEqual({ a: 'string' }); + }); + + it('does not mutate inputs', () => { + const target = { a: { b: 1 } }; + const source = { a: { c: 2 } }; + deepMerge(target, source); + expect(target).toEqual({ a: { b: 1 } }); + expect(source).toEqual({ a: { c: 2 } }); + }); + + it('handles empty source', () => { + expect(deepMerge({ a: 1 }, {})).toEqual({ a: 1 }); + }); + + it('handles empty target', () => { + expect(deepMerge({}, { a: 1 })).toEqual({ a: 1 }); + }); + + it('does not merge arrays', () => { + expect(deepMerge({ a: [1, 2] }, { a: [3, 4] })).toEqual({ a: [3, 4] }); + }); +}); diff --git a/src/lib/utils/__tests__/zod.test.ts b/src/lib/utils/__tests__/zod.test.ts index 264e7fabe..3579dd9cd 100644 --- a/src/lib/utils/__tests__/zod.test.ts +++ b/src/lib/utils/__tests__/zod.test.ts @@ -1,5 +1,6 @@ -import { validateAgentSchema, validateProjectSchema } from '../zod.js'; +import { validateAgentSchema, validateProjectSchema, withCatchAll } from '../zod.js'; import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; describe('validateAgentSchema', () => { const validAgent = { @@ -79,3 +80,51 @@ describe('validateProjectSchema', () => { ).toThrow('Invalid AgentCoreProjectSpec'); }); }); + +describe('withCatchAll', () => { + it('wraps top-level fields with catch', () => { + const strict = z.object({ name: z.string(), age: z.number() }); + const lenient = withCatchAll(strict); + + const result = lenient.parse({ name: 'valid', age: 'not a number' }); + expect(result.name).toBe('valid'); + expect(result.age).toBeUndefined(); + }); + + it('wraps nested object fields with catch', () => { + const strict = z.object({ + settings: z.object({ + enabled: z.boolean(), + name: z.string(), + }), + }); + const lenient = withCatchAll(strict); + + const result = lenient.parse({ settings: { enabled: 'bad', name: 'good' } }); + expect(result.settings.enabled).toBeUndefined(); + expect(result.settings.name).toBe('good'); + }); + + it('handles optional fields', () => { + const strict = z.object({ value: z.string().optional() }); + const lenient = withCatchAll(strict); + + const result = lenient.parse({}); + expect(result.value).toBeUndefined(); + }); + + it('preserves unknown keys via loose', () => { + const strict = z.object({ known: z.string() }); + const lenient = withCatchAll(strict); + + const result = lenient.parse({ known: 'hello', extra: 'world' }) as Record; + expect(result.known).toBe('hello'); + expect(result.extra).toBe('world'); + }); + + it('passes through primitive schemas unchanged', () => { + const schema = z.string(); + const result = withCatchAll(schema); + expect(result.parse('hello')).toBe('hello'); + }); +}); diff --git a/src/lib/utils/index.ts b/src/lib/utils/index.ts index 1ab339321..8168b5a2b 100644 --- a/src/lib/utils/index.ts +++ b/src/lib/utils/index.ts @@ -12,4 +12,4 @@ export { export { parseTimeString } from './time-parser'; export { parseJsonRpcResponse } from './json-rpc'; export { poll, isThrottlingError } from './polling'; -export { validateAgentSchema, validateProjectSchema } from './zod'; +export { validateAgentSchema, validateProjectSchema, withCatchAll } from './zod'; diff --git a/src/lib/utils/object.ts b/src/lib/utils/object.ts new file mode 100644 index 000000000..a33079248 --- /dev/null +++ b/src/lib/utils/object.ts @@ -0,0 +1,15 @@ +export function deepMerge(target: Record, source: Record): Record { + const result = { ...target }; + for (const key of Object.keys(source)) { + if (isPlainObject(result[key]) && isPlainObject(source[key])) { + result[key] = deepMerge(result[key], source[key]); + } else { + result[key] = source[key]; + } + } + return result; +} + +function isPlainObject(val: unknown): val is Record { + return typeof val === 'object' && val !== null && !Array.isArray(val); +} diff --git a/src/lib/utils/zod.ts b/src/lib/utils/zod.ts index f2f13a4f2..0010fc135 100644 --- a/src/lib/utils/zod.ts +++ b/src/lib/utils/zod.ts @@ -1,5 +1,25 @@ import type { AgentCoreProjectSpec, AgentEnvSpec } from '../../schema'; import { AgentCoreProjectSpecSchema, AgentEnvSpecSchema } from '../../schema'; +import { z } from 'zod'; + +/** + * Recursively wraps all fields in a Zod schema with `.catch(undefined)` and + * adds `.loose()` to objects, making parsing lenient — invalid fields + * are silently dropped and unknown keys are preserved. + */ +export function withCatchAll(schema: T): T { + if (schema instanceof z.ZodObject) { + const shape: Record = schema.shape; + const newShape = Object.fromEntries( + Object.entries(shape).map(([key, field]) => [key, withCatchAll(field).catch(undefined)]) + ); + return z.object(newShape).loose() as unknown as T; + } + if (schema instanceof z.ZodOptional) { + return z.optional(withCatchAll(schema.unwrap() as z.ZodType)) as unknown as T; + } + return schema; +} /** * Pass agent spec through zod validator From d45478803484f0a969633544318a354d37de94a4 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 26 May 2026 18:54:24 -0400 Subject: [PATCH 2/4] fix: simplify set path by validing the partial instead of the merged --- src/cli/commands/config/actions.ts | 8 ++--- src/lib/utils/__tests__/object.test.ts | 46 -------------------------- src/lib/utils/object.ts | 15 --------- 3 files changed, 3 insertions(+), 66 deletions(-) delete mode 100644 src/lib/utils/__tests__/object.test.ts delete mode 100644 src/lib/utils/object.ts diff --git a/src/cli/commands/config/actions.ts b/src/cli/commands/config/actions.ts index 0954a81e9..dc72e0582 100644 --- a/src/cli/commands/config/actions.ts +++ b/src/cli/commands/config/actions.ts @@ -1,6 +1,6 @@ import { readGlobalConfig, updateGlobalConfig, validateGlobalConfig } from '../../../lib/schemas/io/global-config.js'; -import { deepMerge } from '../../../lib/utils/object.js'; import type { ConfigResult } from './types.js'; +import { ValidationError } from '@/lib/index.js'; export async function handleConfigList(): Promise { const config = await readGlobalConfig(); @@ -19,13 +19,11 @@ export async function handleConfigGet(key: string): Promise { export async function handleConfigSet(key: string, raw: string): Promise { const value = parseValue(raw); - const existing = await readGlobalConfig(); const partial = buildNestedObject(key, value); - const merged = deepMerge(existing, partial); + const validation = validateGlobalConfig(partial); - const validation = validateGlobalConfig(merged); if (!validation.success) { - return { success: false, error: new Error(`Invalid value "${raw}" for key "${key}".`) }; + return { success: false, error: new ValidationError(`Invalid value "${raw}" for key "${key}".`) }; } const ok = await updateGlobalConfig(partial); diff --git a/src/lib/utils/__tests__/object.test.ts b/src/lib/utils/__tests__/object.test.ts deleted file mode 100644 index 1bccca8e6..000000000 --- a/src/lib/utils/__tests__/object.test.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { deepMerge } from '../object.js'; -import { describe, expect, it } from 'vitest'; - -describe('deepMerge', () => { - it('merges flat objects', () => { - expect(deepMerge({ a: 1 }, { b: 2 })).toEqual({ a: 1, b: 2 }); - }); - - it('overwrites primitive values', () => { - expect(deepMerge({ a: 1 }, { a: 2 })).toEqual({ a: 2 }); - }); - - it('recursively merges nested objects', () => { - const target = { nested: { a: 1, b: 2 } }; - const source = { nested: { b: 3, c: 4 } }; - expect(deepMerge(target, source)).toEqual({ nested: { a: 1, b: 3, c: 4 } }); - }); - - it('overwrites non-object with object', () => { - expect(deepMerge({ a: 'string' }, { a: { nested: true } })).toEqual({ a: { nested: true } }); - }); - - it('overwrites object with non-object', () => { - expect(deepMerge({ a: { nested: true } }, { a: 'string' })).toEqual({ a: 'string' }); - }); - - it('does not mutate inputs', () => { - const target = { a: { b: 1 } }; - const source = { a: { c: 2 } }; - deepMerge(target, source); - expect(target).toEqual({ a: { b: 1 } }); - expect(source).toEqual({ a: { c: 2 } }); - }); - - it('handles empty source', () => { - expect(deepMerge({ a: 1 }, {})).toEqual({ a: 1 }); - }); - - it('handles empty target', () => { - expect(deepMerge({}, { a: 1 })).toEqual({ a: 1 }); - }); - - it('does not merge arrays', () => { - expect(deepMerge({ a: [1, 2] }, { a: [3, 4] })).toEqual({ a: [3, 4] }); - }); -}); diff --git a/src/lib/utils/object.ts b/src/lib/utils/object.ts deleted file mode 100644 index a33079248..000000000 --- a/src/lib/utils/object.ts +++ /dev/null @@ -1,15 +0,0 @@ -export function deepMerge(target: Record, source: Record): Record { - const result = { ...target }; - for (const key of Object.keys(source)) { - if (isPlainObject(result[key]) && isPlainObject(source[key])) { - result[key] = deepMerge(result[key], source[key]); - } else { - result[key] = source[key]; - } - } - return result; -} - -function isPlainObject(val: unknown): val is Record { - return typeof val === 'object' && val !== null && !Array.isArray(val); -} From 6c18be64f81df71008494528c46911ab8a50703a Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 26 May 2026 19:25:59 -0400 Subject: [PATCH 3/4] refactor: unify parsing logic with telemetry for common abstraction --- src/cli/telemetry/cli-command-run.ts | 9 ++- .../schemas/__tests__/command-run.test.ts | 15 ++-- src/cli/telemetry/schemas/common-shapes.ts | 18 ----- src/lib/schemas/io/global-config.ts | 8 +-- src/lib/utils/__tests__/zod.test.ts | 70 +++++++++++-------- src/lib/utils/index.ts | 2 +- src/lib/utils/zod.ts | 65 +++++++++++++---- 7 files changed, 113 insertions(+), 74 deletions(-) diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index d60c0e8fd..f0b4ee84b 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -1,11 +1,12 @@ import type { Result } from '../../lib/result'; +import { resilientParse } from '../../lib/utils/zod.js'; import { getErrorMessage } from '../errors'; import { type AttributeRecorder, createAttributeRecorder } from './attribute-recorder.js'; import { TelemetryClientAccessor } from './client-accessor.js'; import { TelemetryClient } from './client.js'; import { classifyError } from './error.js'; import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from './schemas/command-run.js'; -import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js'; +import { type CommandResult, CommandResultSchema } from './schemas/common-shapes.js'; import { performance } from 'perf_hooks'; export type { AttributeRecorder } from './attribute-recorder.js'; @@ -30,7 +31,11 @@ function recordCommandRun( const validatedAttrs = Object.keys(attrs as Record).length > 0 - ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) + ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record, { + fallback: 'unknown', + fillMissing: true, + keepUnknown: false, + }) : attrs; client.emit('cli.command_run', durationMs, { diff --git a/src/cli/telemetry/schemas/__tests__/command-run.test.ts b/src/cli/telemetry/schemas/__tests__/command-run.test.ts index 561899d7e..5c647b2b0 100644 --- a/src/cli/telemetry/schemas/__tests__/command-run.test.ts +++ b/src/cli/telemetry/schemas/__tests__/command-run.test.ts @@ -1,6 +1,7 @@ +import { resilientParse } from '../../../../lib/utils/zod'; import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from '../command-run'; import { ResourceAttributesSchema } from '../common-attributes'; -import { CommandResultSchema, resilientParse } from '../common-shapes'; +import { CommandResultSchema } from '../common-shapes'; import { describe, expect, expectTypeOf, it } from 'vitest'; import { z } from 'zod'; @@ -237,6 +238,8 @@ describe('type safety', () => { }); }); +const TELEMETRY_OPTS = { fallback: 'unknown', fillMissing: true, keepUnknown: false } as const; + describe('resilientParse', () => { it('passes valid attrs through unchanged', () => { const attrs = { @@ -250,7 +253,7 @@ describe('resilientParse', () => { network_mode: 'public', has_agent: true, }; - expect(resilientParse(COMMAND_SCHEMAS.create, attrs)).toEqual(attrs); + expect(resilientParse(COMMAND_SCHEMAS.create, attrs, TELEMETRY_OPTS)).toEqual(attrs); }); it('defaults a single invalid enum field to unknown', () => { @@ -265,26 +268,26 @@ describe('resilientParse', () => { network_mode: 'public', has_agent: true, }; - const result = resilientParse(COMMAND_SCHEMAS.create, attrs); + const result = resilientParse(COMMAND_SCHEMAS.create, attrs, TELEMETRY_OPTS); expect(result.agent_language).toBe('unknown'); expect(result.agent_framework).toBe('strands'); }); it('defaults missing required fields to unknown', () => { - const result = resilientParse(COMMAND_SCHEMAS.create, { agent_language: 'python' }); + const result = resilientParse(COMMAND_SCHEMAS.create, { agent_language: 'python' }, TELEMETRY_OPTS); expect(result.agent_language).toBe('python'); expect(result.agent_framework).toBe('unknown'); expect(result.model_provider).toBe('unknown'); }); it('defaults all fields to unknown when all are invalid', () => { - const result = resilientParse(COMMAND_SCHEMAS.create, {}); + const result = resilientParse(COMMAND_SCHEMAS.create, {}, TELEMETRY_OPTS); for (const value of Object.values(result)) { expect(value).toBe('unknown'); } }); it('returns empty object for no-attrs schemas', () => { - expect(resilientParse(COMMAND_SCHEMAS['telemetry.disable'], {})).toEqual({}); + expect(resilientParse(COMMAND_SCHEMAS['telemetry.disable'], {}, TELEMETRY_OPTS)).toEqual({}); }); }); diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index e3aa86bad..a0f679a61 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -8,24 +8,6 @@ export function safeSchema>(shape: T) { return z.object(shape); } -/** - * Validate each field in a schema individually, defaulting to 'unknown' on failure. - * This ensures a single invalid attribute never blocks the entire metric from being published. - * Keys in attrs not present in the schema are omitted from the result. - */ -export function resilientParse( - schema: z.ZodObject, - attrs: Record -): Record { - const result: Record = {}; - for (const key of Object.keys(schema.shape)) { - const field = schema.shape[key] as z.ZodType; - const parsed = field.safeParse(attrs[key]); - result[key] = parsed.success ? parsed.data : 'unknown'; - } - return result; -} - /** * Lowercase a CLI value and parse it through a Zod enum, returning the narrowed type. * The `as` cast on the failure branch is intentional: invalid values pass through to diff --git a/src/lib/schemas/io/global-config.ts b/src/lib/schemas/io/global-config.ts index aecced026..5d4c27ee0 100644 --- a/src/lib/schemas/io/global-config.ts +++ b/src/lib/schemas/io/global-config.ts @@ -1,4 +1,4 @@ -import { withCatchAll } from '../../utils/zod.js'; +import { resilientParse } from '../../utils/zod.js'; import { readFileSync } from 'fs'; import { mkdir, readFile, writeFile } from 'fs/promises'; import { randomUUID } from 'node:crypto'; @@ -27,8 +27,6 @@ const GlobalConfigSchemaStrict = z }) .strict(); -const GlobalConfigSchema = withCatchAll(GlobalConfigSchemaStrict); - export type GlobalConfig = z.infer; export function validateGlobalConfig(data: unknown): { success: boolean; error?: z.ZodError } { @@ -38,7 +36,7 @@ export function validateGlobalConfig(data: unknown): { success: boolean; error?: export async function readGlobalConfig(configFile = GLOBAL_CONFIG_FILE): Promise { try { const data = await readFile(configFile, 'utf-8'); - return GlobalConfigSchema.parse(JSON.parse(data)); + return resilientParse(GlobalConfigSchemaStrict, JSON.parse(data) as Record); } catch { return {}; } @@ -47,7 +45,7 @@ export async function readGlobalConfig(configFile = GLOBAL_CONFIG_FILE): Promise export function readGlobalConfigSync(configFile = GLOBAL_CONFIG_FILE): GlobalConfig { try { const data = readFileSync(configFile, 'utf-8'); - return GlobalConfigSchema.parse(JSON.parse(data)); + return resilientParse(GlobalConfigSchemaStrict, JSON.parse(data) as Record); } catch { return {}; } diff --git a/src/lib/utils/__tests__/zod.test.ts b/src/lib/utils/__tests__/zod.test.ts index 3579dd9cd..43085e909 100644 --- a/src/lib/utils/__tests__/zod.test.ts +++ b/src/lib/utils/__tests__/zod.test.ts @@ -1,4 +1,4 @@ -import { validateAgentSchema, validateProjectSchema, withCatchAll } from '../zod.js'; +import { resilientParse, validateAgentSchema, validateProjectSchema } from '../zod.js'; import { describe, expect, it } from 'vitest'; import { z } from 'zod'; @@ -81,50 +81,62 @@ describe('validateProjectSchema', () => { }); }); -describe('withCatchAll', () => { - it('wraps top-level fields with catch', () => { - const strict = z.object({ name: z.string(), age: z.number() }); - const lenient = withCatchAll(strict); +describe('resilientParse', () => { + it('passes valid fields through unchanged', () => { + const schema = z.object({ name: z.string(), age: z.number() }); + const result = resilientParse(schema, { name: 'valid', age: 42 }); + expect(result.name).toBe('valid'); + expect(result.age).toBe(42); + }); - const result = lenient.parse({ name: 'valid', age: 'not a number' }); + it('defaults invalid fields to undefined', () => { + const schema = z.object({ name: z.string(), age: z.number() }); + const result = resilientParse(schema, { name: 'valid', age: 'not a number' }); expect(result.name).toBe('valid'); expect(result.age).toBeUndefined(); }); - it('wraps nested object fields with catch', () => { - const strict = z.object({ + it('recursively parses nested objects', () => { + const schema = z.object({ settings: z.object({ enabled: z.boolean(), name: z.string(), }), }); - const lenient = withCatchAll(strict); - - const result = lenient.parse({ settings: { enabled: 'bad', name: 'good' } }); - expect(result.settings.enabled).toBeUndefined(); - expect(result.settings.name).toBe('good'); + const result = resilientParse(schema, { settings: { enabled: 'bad', name: 'good' } }); + expect(result.settings).toEqual({ name: 'good' }); }); - it('handles optional fields', () => { - const strict = z.object({ value: z.string().optional() }); - const lenient = withCatchAll(strict); - - const result = lenient.parse({}); - expect(result.value).toBeUndefined(); + it('skips keys not present in data', () => { + const schema = z.object({ name: z.string(), age: z.number() }); + const result = resilientParse(schema, { name: 'valid' }); + expect(result).toEqual({ name: 'valid' }); + expect('age' in result).toBe(false); }); - it('preserves unknown keys via loose', () => { - const strict = z.object({ known: z.string() }); - const lenient = withCatchAll(strict); - - const result = lenient.parse({ known: 'hello', extra: 'world' }) as Record; + it('preserves unknown keys', () => { + const schema = z.object({ known: z.string() }); + const result = resilientParse(schema, { known: 'hello', extra: 'world' }); expect(result.known).toBe('hello'); - expect(result.extra).toBe('world'); + expect((result as Record).extra).toBe('world'); + }); + + it('recursively parses nested objects wrapped in ZodOptional', () => { + const schema = z.object({ + settings: z + .object({ + enabled: z.boolean(), + name: z.string(), + }) + .optional(), + }); + const result = resilientParse(schema, { settings: { enabled: 'bad', name: 'good' } }); + expect(result.settings).toEqual({ name: 'good' }); }); - it('passes through primitive schemas unchanged', () => { - const schema = z.string(); - const result = withCatchAll(schema); - expect(result.parse('hello')).toBe('hello'); + it('supports custom fallback value', () => { + const schema = z.object({ name: z.string() }); + const result = resilientParse(schema, { name: 123 }, { fallback: 'unknown' }); + expect(result.name).toBe('unknown'); }); }); diff --git a/src/lib/utils/index.ts b/src/lib/utils/index.ts index 8168b5a2b..c97b6be4d 100644 --- a/src/lib/utils/index.ts +++ b/src/lib/utils/index.ts @@ -12,4 +12,4 @@ export { export { parseTimeString } from './time-parser'; export { parseJsonRpcResponse } from './json-rpc'; export { poll, isThrottlingError } from './polling'; -export { validateAgentSchema, validateProjectSchema, withCatchAll } from './zod'; +export { validateAgentSchema, validateProjectSchema, resilientParse, type ResilientParseOptions } from './zod'; diff --git a/src/lib/utils/zod.ts b/src/lib/utils/zod.ts index 0010fc135..a185c1db5 100644 --- a/src/lib/utils/zod.ts +++ b/src/lib/utils/zod.ts @@ -2,23 +2,62 @@ import type { AgentCoreProjectSpec, AgentEnvSpec } from '../../schema'; import { AgentCoreProjectSpecSchema, AgentEnvSpecSchema } from '../../schema'; import { z } from 'zod'; +export interface ResilientParseOptions { + /** Value to use when a field fails validation. Default: undefined */ + fallback?: string | number | boolean; + /** Include schema keys not present in data, set to fallback value. Default: false */ + fillMissing?: boolean; + /** Preserve keys in data not defined in the schema. Default: true */ + keepUnknown?: boolean; +} + /** - * Recursively wraps all fields in a Zod schema with `.catch(undefined)` and - * adds `.loose()` to objects, making parsing lenient — invalid fields - * are silently dropped and unknown keys are preserved. + * Recursively parse data against a Zod object schema, field by field. + * Invalid fields fall back to a default value rather than throwing. + * Nested ZodObjects (including those wrapped in ZodOptional/ZodNullable/ZodDefault) are parsed recursively. + * + * Note: when keepUnknown is true (default), extra keys are preserved in the result. + * If the result is later validated against a .strict() schema, those keys will cause errors. + * This is intentional for read-path leniency; use validateGlobalConfig for write-path strictness. */ -export function withCatchAll(schema: T): T { - if (schema instanceof z.ZodObject) { - const shape: Record = schema.shape; - const newShape = Object.fromEntries( - Object.entries(shape).map(([key, field]) => [key, withCatchAll(field).catch(undefined)]) - ); - return z.object(newShape).loose() as unknown as T; +export function resilientParse>( + schema: T, + data: Record, + options: ResilientParseOptions = {} +): Partial> { + if (data == null || typeof data !== 'object' || Array.isArray(data)) return {} as Partial>; + const { fallback, fillMissing = false, keepUnknown = true } = options; + const result: Record = {}; + for (const [key, field] of Object.entries(schema.shape)) { + if (!(key in data)) { + if (fillMissing) result[key] = fallback; + continue; + } + const value = data[key]; + const inner = unwrapZodType(field as z.ZodType); + if (inner instanceof z.ZodObject && value != null && typeof value === 'object' && !Array.isArray(value)) { + result[key] = resilientParse(inner, value as Record, options); + } else { + const parsed = (field as z.ZodType).safeParse(value); + result[key] = parsed.success ? parsed.data : fallback; + } + } + if (keepUnknown) { + for (const key of Object.keys(data)) { + if (!(key in schema.shape)) { + result[key] = data[key]; + } + } } - if (schema instanceof z.ZodOptional) { - return z.optional(withCatchAll(schema.unwrap() as z.ZodType)) as unknown as T; + return result as Partial>; +} + +/** Unwrap ZodOptional, ZodNullable, and ZodDefault to get the inner type. */ +function unwrapZodType(field: z.ZodType): z.ZodType { + while (field instanceof z.ZodOptional || field instanceof z.ZodNullable || field instanceof z.ZodDefault) { + field = field.unwrap() as z.ZodType; } - return schema; + return field; } /** From 4d3cef131dcd37435337f15b2fc39ba050e66c98 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 26 May 2026 20:07:52 -0400 Subject: [PATCH 4/4] fix: adjust tests to include quotes for strings --- integ-tests/config.test.ts | 4 ++-- src/lib/utils/zod.ts | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/integ-tests/config.test.ts b/integ-tests/config.test.ts index c9620f845..4b0fe3463 100644 --- a/integ-tests/config.test.ts +++ b/integ-tests/config.test.ts @@ -39,7 +39,7 @@ describe('config command', () => { it('gets a value', async () => { const result = await run(['config', 'uvIndex']); expect(result.exitCode).toBe(0); - expect(result.stdout.trim()).toBe('https://example.com'); + expect(result.stdout.trim()).toBe('"https://example.com"'); }); it('sets a nested value with dot notation', async () => { @@ -51,7 +51,7 @@ describe('config command', () => { it('gets a nested value with dot notation', async () => { const result = await run(['config', 'telemetry.endpoint']); expect(result.exitCode).toBe(0); - expect(result.stdout.trim()).toBe('https://metrics.example.com'); + expect(result.stdout.trim()).toBe('"https://metrics.example.com"'); }); it('gets an object value as JSON', async () => { diff --git a/src/lib/utils/zod.ts b/src/lib/utils/zod.ts index a185c1db5..5eb87cc3c 100644 --- a/src/lib/utils/zod.ts +++ b/src/lib/utils/zod.ts @@ -14,11 +14,8 @@ export interface ResilientParseOptions { /** * Recursively parse data against a Zod object schema, field by field. * Invalid fields fall back to a default value rather than throwing. - * Nested ZodObjects (including those wrapped in ZodOptional/ZodNullable/ZodDefault) are parsed recursively. + * Nested ZodObjects are parsed recursively. * - * Note: when keepUnknown is true (default), extra keys are preserved in the result. - * If the result is later validated against a .strict() schema, those keys will cause errors. - * This is intentional for read-path leniency; use validateGlobalConfig for write-path strictness. */ export function resilientParse>( schema: T,