From 7231c73d8fc25293650eaadd9b4ee3db581549b4 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 15 Jul 2026 23:46:22 +0000 Subject: [PATCH 01/13] feat(config): implement global config to wire up config command --- src/globalConfig/accessor.tsx | 136 +++++++++++++ src/globalConfig/index.tsx | 8 + src/globalConfig/source.tsx | 87 ++++++++ src/globalConfig/types.tsx | 78 ++++++++ src/handlers/config/config.test.tsx | 197 ++++++++++++++++--- src/handlers/config/handler.tsx | 34 +++- src/handlers/harness/exec/exec.test.tsx | 13 +- src/handlers/harness/harness.test.tsx | 7 +- src/handlers/harness/invoke/invoke.test.tsx | 13 +- src/handlers/help.screen.test.tsx | 8 +- src/handlers/index.tsx | 9 +- src/handlers/project/project.test.ts | 8 +- src/handlers/root.test.tsx | 2 + src/index.ts | 22 ++- src/middleware/index.tsx | 1 + src/middleware/withGlobalConfig.tsx | 24 +++ src/middleware/withRegion.test.tsx | 8 +- src/{router/schema.tsx => parsing/index.tsx} | 150 +++++++++----- src/router/args.tsx | 4 +- src/router/flags.tsx | 4 +- src/testing/globalConfig.tsx | 27 +++ src/testing/index.tsx | 1 + src/testing/renderScreen.tsx | 8 +- src/tui/tui.test.tsx | 2 + 24 files changed, 755 insertions(+), 96 deletions(-) create mode 100644 src/globalConfig/accessor.tsx create mode 100644 src/globalConfig/index.tsx create mode 100644 src/globalConfig/source.tsx create mode 100644 src/globalConfig/types.tsx create mode 100644 src/middleware/withGlobalConfig.tsx rename src/{router/schema.tsx => parsing/index.tsx} (52%) create mode 100644 src/testing/globalConfig.tsx diff --git a/src/globalConfig/accessor.tsx b/src/globalConfig/accessor.tsx new file mode 100644 index 000000000..9e5685a79 --- /dev/null +++ b/src/globalConfig/accessor.tsx @@ -0,0 +1,136 @@ +import z, { ZodError } from "zod"; +import { + globalConfigSchema, + type AsyncDataSource, + type GlobalConfigAccessor, + type GlobalConfigKey, + type GlobalConfigShape, +} from "./types"; +import { coerce, inspectZodSchema, formatZodError } from "../parsing"; +import type { Logger } from "../logging"; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function getAtPath(obj: object, path: string): unknown { + return path.split(".").reduce((acc, key) => (isRecord(acc) ? acc[key] : undefined), obj); +} + +function setAtPath( + obj: Record, + path: string, + value: unknown, +): Record { + const keys = path.split("."); + if (keys.length === 0) return obj; + const head = keys[0]!; + if (keys.length === 1) return { ...obj, [head]: value }; + return { + ...obj, + [head]: setAtPath( + isRecord(obj[head]) ? (obj[head] as Record) : {}, + keys.slice(1).join("."), + value, + ), + }; +} + +function buildKeys(schema: z.ZodObject, prefix: string = ""): string[] { + return Object.entries(schema.shape).flatMap(([key, value]) => { + const path = prefix ? `${prefix}.${key}` : key; + const unwrapped = inspectZodSchema(value as z.ZodType).innerType; + return unwrapped instanceof z.ZodObject ? [path, ...buildKeys(unwrapped, path)] : [path]; + }); +} + +function buildSchemas(schema: z.ZodObject, prefix: string = ""): Map { + return Object.entries(schema.shape).reduce((map, [key, value]) => { + const path = prefix ? `${prefix}.${key}` : key; + map.set(path, value as z.ZodType); + const unwrapped = inspectZodSchema(value as z.ZodType).innerType; + if (unwrapped instanceof z.ZodObject) { + for (const [k, v] of buildSchemas(unwrapped, path)) { + map.set(k, v); + } + } + return map; + }, new Map()); +} + +export interface GlobalConfigAccessorConfig { + logger: Logger; + source: AsyncDataSource; +} + +/** + * Creates a {@link GlobalConfigAccessor} backed by the given data source. + * + * The accessor provides typed get/put access to config values using dot-notation + * keys (e.g. `"telemetry.enabled"`), with schema validation and coercion on writes. + * + * @param config - The accessor configuration (file path and optional data source override). + * @returns A {@link GlobalConfigAccessor} instance. + */ +export function createGlobalConfigAccessor( + config: GlobalConfigAccessorConfig, +): GlobalConfigAccessor { + const validKeys = new Set(buildKeys(globalConfigSchema)); + const schemas = buildSchemas(globalConfigSchema); + + config.logger.info(`creating global config accessor`); + const accessor: GlobalConfigAccessor = { + get: async (key) => { + config.logger.child({ key, method: "get" }).debug("reading value from global config"); + const data = await config.source.read(); + return getAtPath(data, key) as never; // cast since key is already typechecked. + }, + + put: async (key, value) => { + const logger = config.logger.child({ key, value, method: "put" }); + logger.debug("writing value to global config"); + + const data = await config.source.read(); + const schema = schemas.get(key); + + if (!schema) { + // TODO: make this a ValidationError (but mark it as a errorSource=client) + // note: this shouldn't happen unless we lie to the typesystem since key is strongly typed above, and map includes all schemas. + logger.error("unable to find schema associated with given key. The key is likely invalid"); + throw new Error( + `unable to put value '${JSON.stringify(value)}' for key '${key}' since key does not exist.`, + ); + } + + try { + const coercedValue = coerce(schema, value); + await config.source.write(setAtPath(data, key, coercedValue)); + // note: cast is safe since write above will fail if not. + return coercedValue as typeof value; + } catch (e) { + const error = e instanceof Error ? e : new Error(String(e)); + logger + .child({ errorName: error.name, errorMessage: error.message }) + .error(`failed to write value to global config`); + + if (e instanceof ZodError) { + // TODO: swap to validation error. + throw new TypeError(formatZodError(e)); + } + throw e; + } + }, + + isValidKey: (key): key is GlobalConfigKey => { + config.logger.child({ key, method: "isValidKey" }).debug(); + return validKeys.has(key); + }, + + toObject: () => { + config.logger.child({ method: "toObject" }).debug("retrieving entire config"); + return config.source.read(); + }, + }; + + return accessor; +} diff --git a/src/globalConfig/index.tsx b/src/globalConfig/index.tsx new file mode 100644 index 000000000..c62ec1d35 --- /dev/null +++ b/src/globalConfig/index.tsx @@ -0,0 +1,8 @@ +export { + type GlobalConfigAccessor, + type AsyncDataSource, + type GlobalConfigShape, + globalConfigSchema, +} from "./types"; +export { createGlobalConfigAccessor } from "./accessor"; +export { createJsonFileDataSource } from "./source"; diff --git a/src/globalConfig/source.tsx b/src/globalConfig/source.tsx new file mode 100644 index 000000000..01f0ba273 --- /dev/null +++ b/src/globalConfig/source.tsx @@ -0,0 +1,87 @@ +import z from "zod"; +import type { AsyncDataSource } from "./types"; +import { readFile, writeFile, mkdir, rename, rm } from "fs/promises"; +import { dirname } from "path"; +import type { Logger } from "../logging"; +import { mapSchema } from "../parsing"; + +export interface JsonFileDataSourceConfig { + filePath: string; + /** + * Describes the data to write to the file if it does not exist or is deleted. + */ + initialData: z.infer; + schema: TSchema; + logger: Logger; +} + +function isNodeError(e: unknown): e is NodeJS.ErrnoException { + return e instanceof Error && "code" in e; +} + +// node fs.write is not atomic, and its possible to leave behind corrupted files. +async function atomicWrite(filePath: string, data: string) { + const tempPath = `${filePath}.${crypto.randomUUID()}.tmp`; + + try { + await writeFile(tempPath, data, "utf8"); + await rename(tempPath, filePath); + } catch (e) { + try { + await rm(tempPath); + } catch { + // best effort + } + throw e; + } +} + +/** + * Creates an {@link AsyncDataSource} backed by a JSON file on disk. + * + * Reads are lenient (invalid fields fall back to `undefined`); writes are schema-strict. + * The file and parent directories are created automatically if missing. + * + * @param config - File path, initial data, Zod schema, and logger. + */ +export function createJsonFileDataSource( + config: JsonFileDataSourceConfig, +): AsyncDataSource> { + const logger = config.logger.child({ filePath: config.filePath }); + logger.info(`creating data source from json file '${config.filePath}'`); + + // note: we cast back to original schema type since adding fallback shouldn't change the produced type. + // using a lenient schema here allows us to read "partially" bad configs and ignore bad entries. + const readSchema = mapSchema(config.schema, (s) => s.catch(undefined)) as typeof config.schema; + + return { + read: async () => { + try { + const raw = await readFile(config.filePath, "utf-8"); + return readSchema.parse(JSON.parse(raw)); + } catch (e) { + // If the file doesn't exist, create one for them with the defaults. + if (isNodeError(e) && e.code === "ENOENT") { + logger.warn(`unable to find json file, creating default`); + await mkdir(dirname(config.filePath), { recursive: true }); + await atomicWrite(config.filePath, JSON.stringify(config.initialData, null, 2)); + return config.initialData; + } + + const error = e instanceof Error ? e : new Error(String(e)); + + logger + .child({ errorName: error.name, errorMessage: error.message }) + .error("failed to read config file"); + throw new Error(`unable to read json file at '${config.filePath}'`); + } + }, + + write: async (data) => { + const parsedData = config.schema.parse(data); + await mkdir(dirname(config.filePath), { recursive: true }); + await atomicWrite(config.filePath, JSON.stringify(parsedData, null, 2)); + return parsedData; + }, + }; +} diff --git a/src/globalConfig/types.tsx b/src/globalConfig/types.tsx new file mode 100644 index 000000000..49ee899c6 --- /dev/null +++ b/src/globalConfig/types.tsx @@ -0,0 +1,78 @@ +import z from "zod"; + +/** + * Builds a union type of all fields and their respective shapes recursively via dot notation. + * Ex. + * { user: { name: string, age: number }} -> { user: { name: string, age: number }} | { "user.name": string } | { "user.age": number } + */ +type ObjectEntryShapes = { + [ + TKey in keyof TObject & string // for each key of the object. + ]: + | { [_ in `${TPrefix}${TKey}`]: TObject[TKey] } // extract out the name of each key + | (NonNullable extends Record // check if the value at the key is an object itself. + ? ObjectEntryShapes, `${TPrefix}${TKey}.`> // if it is recurse + : never); // otherwise we are done +}[keyof TObject & string]; // access values to unnest + +/** + * Given a union of object types, returns a object type describing their intersection. + * Ex. + * { user: { name: string, age: number }} | { "user.name": string } | { "user.age": number } + * -> { user: { name: string, age: number }, "user.name": string, "user.age": number } + */ +type UnionToIntersection = (TUnion extends any ? (x: TUnion) => void : never) extends ( + x: infer I, +) => void + ? I + : never; + +type FlattenObject = UnionToIntersection>; + +/** + * A source of data that follows the given shape. + */ +export interface AsyncDataSource { + read(): Promise; + write(s: unknown): Promise; +} + +/** + * Schema for the global config file. {@link GlobalConfigAccessor} is built from this definition. + * Note: all fields should be optional, missing fields in global config should be overriden with reasonable defaults in the consuming code. + */ +export const globalConfigSchema = z.object({ + telemetry: z + .object({ + enabled: z.boolean().optional(), + endpoint: z.string().optional(), + }) + .optional(), + installationId: z.uuid().optional(), +}); + +/** + * Shape of the global config built from {@link globalConfigSchema} + */ +export type GlobalConfigShape = z.infer; + +/** + * Flattened global config shape that leverages dot notation instead of nested objects. + */ +type FlattenedGlobalConfigShape = FlattenObject; + +export type GlobalConfigKey = keyof FlattenedGlobalConfigShape & string; +export type GlobalConfigValue = FlattenedGlobalConfigShape[TKey]; +/** + * Interface for setting and retrieving information from the global configuration for the CLI. + * ex. telemetry settings. + */ +export interface GlobalConfigAccessor { + get(key: TKey): Promise | undefined>; + put( + key: TKey, + value: GlobalConfigValue, + ): Promise>; + isValidKey(key: string): key is GlobalConfigKey; + toObject(): Promise; +} diff --git a/src/handlers/config/config.test.tsx b/src/handlers/config/config.test.tsx index bdb79908c..0ac595621 100644 --- a/src/handlers/config/config.test.tsx +++ b/src/handlers/config/config.test.tsx @@ -1,36 +1,189 @@ -import { test, expect, describe } from "bun:test"; +import { test, expect, describe, beforeEach, afterEach } from "bun:test"; +import { join } from "node:path"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; import { createRootHandler } from "../index"; import { createSilentLogger, TestCoreClient, testIO } from "../../testing"; +import { + createGlobalConfigAccessor, + globalConfigSchema, + createJsonFileDataSource, +} from "../../globalConfig"; -// End-to-end tests for the `config` command, driven through the real root -// handler and top-level route(). A TestCoreClient stands in for Core (config -// doesn't touch it, but createRootHandler requires one); an in-memory io -// captures the output. +describe("config", () => { + let tempDir: string; + let configPath: string; + + const validConfig = { + telemetry: { enabled: true, endpoint: "https://example.com" }, + installationId: "550e8400-e29b-41d4-a716-446655440000", + }; + + const initialConfig = { + installationId: "650e8400-e29b-41d4-a716-446655440000", + }; -async function run(args: string[]): Promise { - const io = testIO(); - const root = createRootHandler(new TestCoreClient(), { - io: io.io, - logger: createSilentLogger(), + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "agentcore-config-test-")); + configPath = join(tempDir, "config.json"); + await writeFile(configPath, JSON.stringify(validConfig)); }); - await root.route(["node", "agentcore", "config", ...args]); - return io.stdout(); -} -describe("config", () => { - test("prints the key and value it was given", async () => { - expect(await run(["telemetry.enabled", "true"])).toBe( - "updating config key=telemetry.enabled, value=true", + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); + }); + + async function run(args: string[]): Promise { + const io = testIO(); + const logger = createSilentLogger(); + const globalConfigAccessor = createGlobalConfigAccessor({ + logger, + source: createJsonFileDataSource({ + filePath: configPath, + schema: globalConfigSchema, + initialData: initialConfig, + logger, + }), + }); + const root = createRootHandler(new TestCoreClient(), { + io: io.io, + logger, + globalConfigAccessor, + }); + await root.route(["node", "agentcore", "config", ...args]); + return io.stdout(); + } + + test("prints the full config when no args are passed", async () => { + const output = await run([]); + expect(JSON.parse(output)).toEqual(validConfig); + }); + + test("prints the value at a key when only a key is passed", async () => { + const output = await run(["telemetry.enabled"]); + expect(JSON.parse(output)).toBe(true); + }); + + test("prints a nested object when a branch key is passed", async () => { + const output = await run(["telemetry"]); + expect(JSON.parse(output)).toEqual(validConfig.telemetry); + }); + + test("sets a value when key and value are passed", async () => { + const initialReadValueOutput = await run(["telemetry.endpoint"]); + expect(JSON.parse(initialReadValueOutput)).toBe(validConfig.telemetry.endpoint); + + const newEndpoint = "http://example2.com"; + + const writeValueOutput = await run(["telemetry.endpoint", newEndpoint]); + expect(JSON.parse(writeValueOutput)).toBe(newEndpoint); + + const readValueOutput = await run(["telemetry.endpoint"]); + expect(JSON.parse(readValueOutput)).toBe(newEndpoint); + }); + + test("accepts json values for non-leaf nodes", async () => { + const initialTelemetrySettingsOutput = await run(["telemetry"]); + const initialTelemetrySettings = JSON.parse(initialTelemetrySettingsOutput); + + const newTelemetrySettings = { + ...initialTelemetrySettings, + enabled: !initialTelemetrySettings.enabled, + }; + const newTelemetrySettingsOutput = await run([ + "telemetry", + JSON.stringify(newTelemetrySettings), + ]); + expect(JSON.parse(newTelemetrySettingsOutput)).toMatchObject(newTelemetrySettings); + + const finalTelemetrySettingsOutput = await run(["telemetry"]); + expect(JSON.parse(finalTelemetrySettingsOutput)).toMatchObject(newTelemetrySettings); + }); + + test("throws on invalid key", async () => { + // TODO: swap to validation error. + expect(run(["nonexistent.key"])).rejects.toThrow(TypeError); + }); + + test("throws on invalid value for key", async () => { + // TODO: swap to validation error + expect(run(["telemetry.enabled", "banana"])).rejects.toThrow(TypeError); + }); + + test("coerces values based on schema", async () => { + const setEnabledOutput = await run(["telemetry.enabled", "false"]); + expect(JSON.parse(setEnabledOutput)).toBe(false); + const readEnabledOutput = await run(["telemetry.enabled"]); + expect(JSON.parse(readEnabledOutput)).toBe(false); + + const setEndpointOutput = await run(["telemetry.endpoint", "false"]); + expect(JSON.parse(setEndpointOutput)).toBe("false"); + const readEndpointOutput = await run(["telemetry.endpoint"]); + expect(JSON.parse(readEndpointOutput)).toBe("false"); + }); + + test("tolerant of partially invalid config files", async () => { + const invalidConfig = { + ...validConfig, + installationId: "purple", + }; + await writeFile(configPath, JSON.stringify(invalidConfig)); + + const fullReadOutput = await run([]); + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { installationId, ...rest } = invalidConfig; + expect(JSON.parse(fullReadOutput)).toEqual(rest); + + const partialReadout = await run(["installationId"]); + expect(partialReadout).toEqual(""); + + const uuid = crypto.randomUUID(); + const writeOutput = await run(["installationId", uuid]); + expect(JSON.parse(writeOutput)).toBe(uuid); + + const readOutput = await run(["installationId"]); + expect(readOutput).toEqual(`"${uuid}"`); + }); + + test("tolerates a single bad field inside a nested object without losing sibling fields", async () => { + await writeFile( + configPath, + JSON.stringify({ + telemetry: { enabled: "not-a-bool", endpoint: "https://good.com" }, + }), ); + const output = await run(["telemetry.endpoint"]); + expect(JSON.parse(output)).toBe("https://good.com"); }); - test("value is undefined when only a key is passed", async () => { - expect(await run(["telemetry.enabled"])).toBe( - "updating config key=telemetry.enabled, value=undefined", + test("forward compatible with unsupported fields", async () => { + await writeFile( + configPath, + JSON.stringify({ + ...validConfig, + futureField: "unknown", + telemetry: { ...validConfig.telemetry, futureNested: 42 }, + }), ); + const output = await run([]); + expect(JSON.parse(output)).toEqual(validConfig); + }); + + test("throws clear error on invalid json", async () => { + await writeFile(configPath, "not { valid json"); + // TODO: assert on error type + expect(run([])).rejects.toThrow("unable to read"); }); - test("both key and value are undefined when none are passed", async () => { - expect(await run([])).toBe("updating config key=undefined, value=undefined"); + test("creates default config if it does not exist", async () => { + await rm(configPath); + const newEndpoint = "http://new-endpoint.command"; + + const output = await run(["telemetry.endpoint", newEndpoint]); + expect(JSON.parse(output)).toBe(newEndpoint); + + const readOutput = await run(["telemetry.endpoint"]); + expect(JSON.parse(readOutput)).toBe(newEndpoint); }); }); diff --git a/src/handlers/config/handler.tsx b/src/handlers/config/handler.tsx index 3cdbeaebb..856977f37 100644 --- a/src/handlers/config/handler.tsx +++ b/src/handlers/config/handler.tsx @@ -1,6 +1,7 @@ import z from "zod"; import { createHandler, argument } from "../../router"; -import type { AppIO } from "../types"; +import { GlobalConfigKey } from "../../middleware"; +import { JsonRendererKey } from "../../tui"; /* * read/write global configuration values. ex. telemetry settings, log level, etc. @@ -9,7 +10,7 @@ import type { AppIO } from "../types"; * `config [key]` prints the value with key. * `config` prints the full config. */ -export const createConfigHandler = (io: AppIO) => +export const createConfigHandler = () => createHandler({ name: "config", description: "read/write global config values", @@ -21,8 +22,31 @@ export const createConfigHandler = (io: AppIO) => ), argument("value", "value to set for the key", z.string().optional()), ], - handle: async (_ctx, _flags, args) => { - // TODO: implment a real handler that update the global config via injected config accessor. - io.stdout.write(`updating config key=${args.key}, value=${args.value}\n`); + handle: async (ctx, _flags, args) => { + const globalConfigAccessor = ctx.require(GlobalConfigKey); + const jsonRenderer = ctx.require(JsonRendererKey); + + // print entire config when key is missing. + if (!args.key) { + const fullConfig = await globalConfigAccessor.toObject(); + if (fullConfig !== undefined) jsonRenderer.renderJson(fullConfig); + return; + } + + const configKey = args.key; + if (!globalConfigAccessor.isValidKey(configKey)) { + // TODO: make this a validation error. + throw new TypeError(`config key '${configKey}' does not exist.`); + } + + // print entire value at key when value is missing. + if (!args.value) { + const scopedConfig = await globalConfigAccessor.get(configKey); + if (scopedConfig !== undefined) jsonRenderer.renderJson(scopedConfig); + return; + } + // update value at key with given value. + const newValue = await globalConfigAccessor.put(configKey, args.value); + jsonRenderer.renderJson(newValue); }, }); diff --git a/src/handlers/harness/exec/exec.test.tsx b/src/handlers/harness/exec/exec.test.tsx index bb548a98d..10ca89e5f 100644 --- a/src/handlers/harness/exec/exec.test.tsx +++ b/src/handlers/harness/exec/exec.test.tsx @@ -5,7 +5,12 @@ import type { } from "@aws-sdk/client-bedrock-agentcore"; import type { GetHarnessResponse } from "@aws-sdk/client-bedrock-agentcore-control"; import { createRootHandler } from "../../index"; -import { createSilentLogger, TestCoreClient, testIO } from "../../../testing"; +import { + createSilentLogger, + TestCoreClient, + testIO, + testGlobalConfigAccessor, +} from "../../../testing"; // Command-flow tests for `harness exec`, driven through the real root handler. // Like the invoke suite, these use a TestCoreClient because the command @@ -31,7 +36,11 @@ async function run(args: string[], configure?: (core: TestCoreClient) => void) { core.harness.setExecEvents(...EXEC_EVENTS); configure?.(core); const io = testIO(); - const root = createRootHandler(core, { io: io.io, logger: createSilentLogger() }); + const root = createRootHandler(core, { + io: io.io, + logger: createSilentLogger(), + globalConfigAccessor: testGlobalConfigAccessor(), + }); await root.route(["node", "agentcore", ...args, "--region", "us-west-2"]); return { core, stdout: io.stdout() }; } diff --git a/src/handlers/harness/harness.test.tsx b/src/handlers/harness/harness.test.tsx index 59a8a11a0..c5e9cda27 100644 --- a/src/handlers/harness/harness.test.tsx +++ b/src/handlers/harness/harness.test.tsx @@ -8,6 +8,7 @@ import { isRecording, matchGolden, testIO, + testGlobalConfigAccessor, } from "../../testing"; // End-to-end command-flow tests for the `harness` subtree. @@ -32,7 +33,11 @@ async function run(args: string[]): Promise { const { createControlClient, createDataClient, createIamClient } = fixtureFactories(FIXTURES); const core = new CoreClient(createControlClient, createDataClient, createIamClient); const io = testIO(); - const root = createRootHandler(core, { io: io.io, logger: createSilentLogger() }); + const root = createRootHandler(core, { + io: io.io, + logger: createSilentLogger(), + globalConfigAccessor: testGlobalConfigAccessor(), + }); await root.route(["node", "agentcore", ...args, "--region", REGION]); return io.stdout(); } diff --git a/src/handlers/harness/invoke/invoke.test.tsx b/src/handlers/harness/invoke/invoke.test.tsx index 25015719a..784404706 100644 --- a/src/handlers/harness/invoke/invoke.test.tsx +++ b/src/handlers/harness/invoke/invoke.test.tsx @@ -5,7 +5,12 @@ import type { } from "@aws-sdk/client-bedrock-agentcore"; import type { GetHarnessResponse } from "@aws-sdk/client-bedrock-agentcore-control"; import { createRootHandler } from "../../index"; -import { createSilentLogger, TestCoreClient, testIO } from "../../../testing"; +import { + createSilentLogger, + TestCoreClient, + testIO, + testGlobalConfigAccessor, +} from "../../../testing"; // Command-flow tests for `harness invoke`, driven through the real root handler // exactly as the CLI runs it. Unlike the get/list suites these use a @@ -41,7 +46,11 @@ async function run(args: string[], configure?: (core: TestCoreClient) => void) { core.harness.setInvokeEvents(...TURN_EVENTS); configure?.(core); const io = testIO(); - const root = createRootHandler(core, { io: io.io, logger: createSilentLogger() }); + const root = createRootHandler(core, { + io: io.io, + logger: createSilentLogger(), + globalConfigAccessor: testGlobalConfigAccessor(), + }); await root.route(["node", "agentcore", ...args, "--region", "us-west-2"]); return { core, stdout: io.stdout() }; } diff --git a/src/handlers/help.screen.test.tsx b/src/handlers/help.screen.test.tsx index 064c68adf..bab0064e8 100644 --- a/src/handlers/help.screen.test.tsx +++ b/src/handlers/help.screen.test.tsx @@ -4,7 +4,7 @@ import { render, cleanup } from "ink-testing-library"; import { ValueContext, compile, CommandKey } from "../router"; import { createRootHandler } from "./index"; import { HelpScreen } from "./screen"; -import { createSilentLogger, TestCoreClient, testIO } from "../testing"; +import { createSilentLogger, TestCoreClient, testIO, testGlobalConfigAccessor } from "../testing"; afterEach(cleanup); @@ -16,7 +16,11 @@ afterEach(cleanup); describe("HelpScreen", () => { test("renders the command's help text", () => { const command = compile( - createRootHandler(new TestCoreClient(), { io: testIO().io, logger: createSilentLogger() }), + createRootHandler(new TestCoreClient(), { + io: testIO().io, + logger: createSilentLogger(), + globalConfigAccessor: testGlobalConfigAccessor(), + }), ValueContext.EmptyContext(), ); const ctx = ValueContext.EmptyContext().withValue(CommandKey, command); diff --git a/src/handlers/index.tsx b/src/handlers/index.tsx index 3707fc373..b0a26e187 100644 --- a/src/handlers/index.tsx +++ b/src/handlers/index.tsx @@ -5,13 +5,15 @@ import { DebugKey, EndpointKey, JsonKey, RegionKey } from "./keys.tsx"; import { createConfigHandler } from "./config/"; import { createProjectHandler } from "./project/index.ts"; import { renderTui } from "../tui"; -import { withRegion, withJsonRenderer, withLogging } from "../middleware"; +import { withRegion, withJsonRenderer, withLogging, withGlobalConfig } from "../middleware"; import type { AppIO, Core } from "./types.tsx"; import type { Logger } from "../logging"; +import type { GlobalConfigAccessor } from "../globalConfig"; export interface RootHandlerConfig { io: AppIO; logger: Logger; + globalConfigAccessor: GlobalConfigAccessor; } export function createRootHandler(core: Core, config: RootHandlerConfig): Router { @@ -32,10 +34,13 @@ export function createRootHandler(core: Core, config: RootHandlerConfig): Router // Inject a logger into each handler. root.use(withLogging({ logger })); + // Pin the global config accessor on the context for any handler that needs it. + root.use(withGlobalConfig(config.globalConfigAccessor)); + // Install sub handlers root.handler(createHarnessHandler(core, io)); root.handler(createRuntimeHandler(core, io)); - root.handler(createConfigHandler(io)); + root.handler(createConfigHandler()); root.handler(createProjectHandler()); // Invoking with no subcommand launches the interactive TUI. diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index 6c2922920..10067936c 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -1,11 +1,17 @@ import { test, expect, describe } from "bun:test"; import { createRootHandler } from "../index"; -import { createSilentLogger, TestCoreClient, testIO } from "../../testing"; +import { + createSilentLogger, + TestCoreClient, + testGlobalConfigAccessor, + testIO, +} from "../../testing"; async function run(args: string[]): Promise { const io = testIO(); const root = createRootHandler(new TestCoreClient(), { io: io.io, + globalConfigAccessor: testGlobalConfigAccessor(), logger: createSilentLogger(), }); await root.route(["node", "agentcore", "project", ...args]); diff --git a/src/handlers/root.test.tsx b/src/handlers/root.test.tsx index b79eaec2f..1c7817420 100644 --- a/src/handlers/root.test.tsx +++ b/src/handlers/root.test.tsx @@ -1,12 +1,14 @@ import { test, expect, describe } from "bun:test"; import { createRootHandler } from "./index"; import { createSilentLogger, TestCoreClient, testIO } from "../testing"; +import { testGlobalConfigAccessor } from "../testing"; describe("createRootHandler", () => { test("builds the agentcore command tree with its subcommands", () => { const root = createRootHandler(new TestCoreClient(), { io: testIO().io, logger: createSilentLogger(), + globalConfigAccessor: testGlobalConfigAccessor(), }); expect(root.name()).toBe("agentcore"); expect(root.children().map((c) => c.name())).toEqual([ diff --git a/src/index.ts b/src/index.ts index bb41c6042..eaf80d42d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,11 @@ import { createControlClient, createDataClient, createIamClient } from "./core/f import { createRootHandler } from "./handlers"; import { createFileLogger, LOG_LEVEL } from "./logging"; import { runWithExitCode } from "./runnable"; +import { + createGlobalConfigAccessor, + globalConfigSchema, + createJsonFileDataSource, +} from "./globalConfig"; process.exit( await runWithExitCode(async (argv: string[]) => { @@ -21,7 +26,6 @@ process.exit( const rootLogger = createFileLogger({ filePath: join(homedir(), ".agentcore", "logs", "output"), - // TODO: allow overriding via global settings logLevel: LOG_LEVEL.DEBUG, bindings: { cliSessionId }, }); @@ -33,7 +37,20 @@ process.exit( }; try { - // Wrap the SDK clients in the CoreClient the handlers consume. Passing + const globalConfigAccessor = createGlobalConfigAccessor({ + logger: rootLogger.child({ module: "globalConfigAccessor" }), + source: createJsonFileDataSource({ + filePath: join(homedir(), ".agentcore", "config.json"), + schema: globalConfigSchema, + initialData: { installationId: crypto.randomUUID() }, + logger: rootLogger.child({ module: "jsonDataSource" }), + }), + }); + + // TODO: get or create an installationId here that we can write into telemetry + + rootLogger.info(`running CLI`); + // factories (rather than instances) lets CoreClient build one client per // region on demand. const coreClient = new CoreClient(createControlClient, createDataClient, createIamClient); @@ -44,6 +61,7 @@ process.exit( const rootHandler = createRootHandler(coreClient, { io, logger: rootLogger, + globalConfigAccessor, }); // Handle the request diff --git a/src/middleware/index.tsx b/src/middleware/index.tsx index e4f9a09f3..ac3ab04d8 100644 --- a/src/middleware/index.tsx +++ b/src/middleware/index.tsx @@ -2,3 +2,4 @@ export { withRegion } from "./withRegion"; export { withTuiOnEmptyFlagsAndArgs } from "./withTuiOnEmptyFlagsAndArgs"; export { withJsonRenderer } from "./withJsonRenderer"; export { withLogging } from "./withLogging"; +export { withGlobalConfig, GlobalConfigKey } from "./withGlobalConfig"; diff --git a/src/middleware/withGlobalConfig.tsx b/src/middleware/withGlobalConfig.tsx new file mode 100644 index 000000000..4c47e4904 --- /dev/null +++ b/src/middleware/withGlobalConfig.tsx @@ -0,0 +1,24 @@ +import type { Middleware } from "../router"; +import { contextKey, type ContextKey } from "../router"; +import type { GlobalConfigAccessor } from "../globalConfig"; + +export const GlobalConfigKey: ContextKey = + contextKey("globalConfig"); + +/** + * Middleware that pins a GlobalConfigAccessor on the context, making it + * available to every handler beneath the mount point via + * `ctx.require(GlobalConfigKey)`. + */ +export function withGlobalConfig(accessor: GlobalConfigAccessor): Middleware { + return (h) => ({ + name: () => h.name(), + description: () => h.description(), + flags: () => h.flags(), + arguments: () => h.arguments(), + children: () => h.children(), + handle: async (ctx, flags, args) => { + await h.handle(ctx.withValue(GlobalConfigKey, accessor), flags, args); + }, + }); +} diff --git a/src/middleware/withRegion.test.tsx b/src/middleware/withRegion.test.tsx index 93a15d8ac..46c97c6c9 100644 --- a/src/middleware/withRegion.test.tsx +++ b/src/middleware/withRegion.test.tsx @@ -3,7 +3,7 @@ import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createRootHandler } from "../handlers"; -import { TestCoreClient, testIO } from "../testing"; +import { TestCoreClient, testGlobalConfigAccessor, testIO } from "../testing"; import { createSilentLogger } from "../testing/"; // writeConfigFile writes an AWS shared-config file with the given contents to a @@ -24,7 +24,11 @@ function writeConfigFile(contents: string): string { // opening the TUI) and returns the region the handler passed to Core. async function resolvedRegion(args: string[]): Promise { const core = new TestCoreClient(); - const root = createRootHandler(core, { io: testIO().io, logger: createSilentLogger() }); + const root = createRootHandler(core, { + io: testIO().io, + logger: createSilentLogger(), + globalConfigAccessor: testGlobalConfigAccessor(), + }); await root.route(["node", "agentcore", "harness", "list", "--json", ...args]); const call = core.harness.calls.at(-1); const options = call?.args[2] as { region: string }; diff --git a/src/router/schema.tsx b/src/parsing/index.tsx similarity index 52% rename from src/router/schema.tsx rename to src/parsing/index.tsx index d89608457..eda22ea2a 100644 --- a/src/router/schema.tsx +++ b/src/parsing/index.tsx @@ -1,14 +1,51 @@ -import type z from "zod"; +import type { ZodType } from "zod"; +import z from "zod"; -// Inspection describes how a zod schema maps onto a Commander option: whether a -// value must be supplied (required), whether it is variadic (an array), whether -// it is a boolean toggle, and any default value. -interface Inspection { - required: boolean; - variadic: boolean; - boolean: boolean; - hasDefault: boolean; - defaultValue: unknown; +export function formatZodError(error: z.ZodError): string { + return error.issues.map((issue) => issue.message).join("; "); +} + +/** + * Converts a raw value into the type the schema expects. + * + * @param schema - The zod schema describing the expected type. + * @param raw - The raw value + * @returns the coerced value matching the schema's expected type or the original value if unable to coerce + */ +export function coerce(schema: z.ZodType, raw: unknown): unknown { + if (raw === undefined) return raw; // defer to schema default / optionality + if (typeof raw === "boolean") return raw; // boolean toggles are already parsed + const type = baseType(schema); + if (Array.isArray(raw)) { + return raw.map((r) => coerceScalar(type, String(r))); + } + if (type === "object") { + return JSON.parse(String(raw)); + } + return coerceScalar(type, String(raw)); +} + +function coerceScalar(type: string | undefined, raw: string): unknown { + switch (type) { + case "number": + return Number(raw); + case "bigint": + try { + return BigInt(raw); + } catch { + return raw; // let zod produce the validation error + } + case "boolean": { + const v = raw.toLowerCase(); + if (v === "true" || v === "1" || v === "yes") return true; + if (v === "false" || v === "0" || v === "no") return false; + return raw; + } + case "date": + return new Date(raw); + default: + return raw; + } } // zod v4 exposes a `.def` with a `type` discriminant and wrapper-specific fields @@ -32,9 +69,26 @@ function baseType(schema: z.ZodType): string | undefined { } } -// inspect peels optional/default/nullable/readonly wrappers off a schema to -// determine the option shape and any default. -export function inspect(schema: z.ZodType): Inspection { +/** + * Describes attributes of a zod schema, such as if its required, hasDefaults, and its innerType. + */ +export interface ZodSchemaInspection { + required: boolean; + variadic: boolean; + boolean: boolean; + hasDefault: boolean; + defaultValue: unknown; + innerType: z.ZodType; +} + +/** + * Peels optional/default/nullable/readonly wrappers off a schema to determine + * the option shape, any default, and the underlying inner type. + * + * @param schema - The zod schema to inspect. + * @returns An inspection describing the schema's structure. + */ +export function inspectZodSchema(schema: z.ZodType): ZodSchemaInspection { const required = !schema.isOptional(); let hasDefault = false; let defaultValue: unknown = undefined; @@ -57,44 +111,42 @@ export function inspect(schema: z.ZodType): Inspection { } } - return { required, variadic, boolean: baseType(schema) === "boolean", hasDefault, defaultValue }; + return { + required, + variadic, + boolean: baseType(schema) === "boolean", + hasDefault, + defaultValue, + innerType: s as ZodType, + }; } -function coerceScalar(type: string | undefined, raw: string): unknown { - switch (type) { - case "number": - return Number(raw); - case "bigint": - try { - return BigInt(raw); - } catch { - return raw; // let zod produce the validation error - } - case "boolean": { - const v = raw.toLowerCase(); - if (v === "true" || v === "1" || v === "yes") return true; - if (v === "false" || v === "0" || v === "no") return false; - return raw; +/** + * Recursively applies `fn` to every leaf schema in a zod object tree, preserving + * wrapper types (optional, nullable, default, readonly) on the way back up. + * + * @param schema - The zod schema to traverse. + * @param fn - Transformation applied to each leaf (and each object after its shape is mapped). + */ +export function mapSchema(schema: z.ZodType, fn: (s: z.ZodType) => z.ZodType): z.ZodType { + const s = schema as unknown as AnyDef; + const t = s.def?.type; + if (t === "object" && "shape" in schema) { + const obj = schema as z.ZodObject; + const newShape: Record = {}; + for (const [key, value] of Object.entries(obj.shape)) { + newShape[key] = mapSchema(value as z.ZodType, fn); } - case "date": - return new Date(raw); - default: - return raw; - } -} - -// coerce converts the raw Commander value (a string, string[] for variadic -// options, or a boolean for toggles) into the type the schema expects. -export function coerce(schema: z.ZodType, raw: unknown): unknown { - if (raw === undefined) return raw; // defer to schema default / optionality - if (typeof raw === "boolean") return raw; // boolean toggles are already parsed - const type = baseType(schema); - if (Array.isArray(raw)) { - return raw.map((r) => coerceScalar(type, String(r))); + return fn(z.object(newShape)); + } else if (t === "optional") { + return mapSchema(s.def!.innerType as unknown as z.ZodType, fn).optional(); + } else if (t === "nullable") { + return mapSchema(s.def!.innerType as unknown as z.ZodType, fn).nullable(); + } else if (t === "default") { + return mapSchema(s.def!.innerType as unknown as z.ZodType, fn).default(s.def!.defaultValue); + } else if (t === "readonly") { + return mapSchema(s.def!.innerType as unknown as z.ZodType, fn).readonly(); + } else { + return fn(schema); } - return coerceScalar(type, String(raw)); -} - -export function formatZodError(error: z.ZodError): string { - return error.issues.map((issue) => issue.message).join("; "); } diff --git a/src/router/args.tsx b/src/router/args.tsx index dd8693eb1..f376d2ea4 100644 --- a/src/router/args.tsx +++ b/src/router/args.tsx @@ -1,9 +1,9 @@ import { type Command, Argument as CommanderArgument } from "commander"; import type { Argument } from "./handler"; -import { coerce, formatZodError, inspect } from "./schema"; +import { coerce, formatZodError, inspectZodSchema } from "../parsing"; export function toCommanderArgument(arg: Argument): CommanderArgument { - const info = inspect(arg.schema); + const info = inspectZodSchema(arg.schema); // Commander treats <> as required and [] as optional: https://github.com/tj/commander.js#more-configuration-1 // Variadic arguments use trailing ... syntax: https://github.com/tj/commander.js#command-arguments const inner = info.variadic ? `${arg.name}...` : arg.name; diff --git a/src/router/flags.tsx b/src/router/flags.tsx index 07fb55c91..07cb5ba6e 100644 --- a/src/router/flags.tsx +++ b/src/router/flags.tsx @@ -1,13 +1,13 @@ import { Option } from "commander"; import type { Context } from "./context"; import type { Flag, GlobalFlag } from "./handler"; -import { coerce, formatZodError, inspect } from "./schema"; +import { coerce, formatZodError, inspectZodSchema } from "../parsing"; // toOption builds a Commander Option from a flag's schema. Booleans become value-less // toggles; everything else takes a value (`` / variadic ``). A // required, non-boolean flag is made mandatory; defaults are forwarded. export function toOption(flag: Flag): Option { - const info = inspect(flag.schema); + const info = inspectZodSchema(flag.schema); const long = `--${flag.name}`; let token: string; diff --git a/src/testing/globalConfig.tsx b/src/testing/globalConfig.tsx new file mode 100644 index 000000000..2c2b907a0 --- /dev/null +++ b/src/testing/globalConfig.tsx @@ -0,0 +1,27 @@ +import type { GlobalConfigAccessor, GlobalConfigShape, AsyncDataSource } from "../globalConfig"; +import { createGlobalConfigAccessor, globalConfigSchema } from "../globalConfig"; +import type { Logger } from "../logging"; +import { createSilentLogger } from "./logging"; + +interface TestGlobalConfigAccessorOptions { + initialConfigData?: GlobalConfigShape; + logger?: Logger; +} + +/** + * In-memory GlobalConfigAccessor for tests. Injects a plain-object + * AsyncDataSource into the real createGlobalConfigAccessor + */ +export function testGlobalConfigAccessor( + options?: TestGlobalConfigAccessorOptions, +): GlobalConfigAccessor { + let data = structuredClone(options?.initialConfigData ?? { installationId: crypto.randomUUID() }); + const source: AsyncDataSource = { + read: async () => data, + write: async (d) => { + data = globalConfigSchema.parse(d); + return data; + }, + }; + return createGlobalConfigAccessor({ source, logger: options?.logger ?? createSilentLogger() }); +} diff --git a/src/testing/index.tsx b/src/testing/index.tsx index 30ee08187..9a60d1817 100644 --- a/src/testing/index.tsx +++ b/src/testing/index.tsx @@ -13,3 +13,4 @@ export { type RenderScreenResult, } from "./renderScreen"; export { createSilentLogger, assertLogsMatch, type LogQuery } from "./logging"; +export { testGlobalConfigAccessor } from "./globalConfig"; diff --git a/src/testing/renderScreen.tsx b/src/testing/renderScreen.tsx index 18973cce1..daf581ceb 100644 --- a/src/testing/renderScreen.tsx +++ b/src/testing/renderScreen.tsx @@ -1,4 +1,3 @@ -import React from "react"; import { render, cleanup } from "ink-testing-library"; import { QueryClient } from "@tanstack/react-query"; import { ValueContext, compile, CommandKey, type Context } from "../router"; @@ -10,6 +9,7 @@ import { TestCoreClient } from "./TestCoreClient"; import { testIO } from "./testIO"; import { tick, waitFor } from "./timing"; import { createSilentLogger } from "./logging"; +import { testGlobalConfigAccessor } from "./globalConfig"; // TUI test harness. // @@ -29,7 +29,11 @@ import { createSilentLogger } from "./logging"; // keeps the command menus faithful to the production command structure. function baseContext(core: TestCoreClient): Context { const rootCommand = compile( - createRootHandler(core, { io: testIO().io, logger: createSilentLogger() }), + createRootHandler(core, { + io: testIO().io, + logger: createSilentLogger(), + globalConfigAccessor: testGlobalConfigAccessor(), + }), ValueContext.EmptyContext(), ); diff --git a/src/tui/tui.test.tsx b/src/tui/tui.test.tsx index c018fb748..93e6a239a 100644 --- a/src/tui/tui.test.tsx +++ b/src/tui/tui.test.tsx @@ -2,6 +2,7 @@ import { test, expect, describe } from "bun:test"; import { createRootHandler } from "../handlers"; import { renderJson } from "./index"; import { createSilentLogger, TestCoreClient, testIO } from "../testing"; +import { testGlobalConfigAccessor } from "../testing"; describe("renderJson", () => { test("pretty-prints a value as indented JSON to the given writer", () => { @@ -21,6 +22,7 @@ describe("--json short-circuits the TUI", () => { const root = createRootHandler(new TestCoreClient(), { io: io.io, logger: createSilentLogger(), + globalConfigAccessor: testGlobalConfigAccessor(), }); await root.route(["node", "agentcore", ...args, "--json"]); return io.stdout(); From 29925ea83a55b86010ad8fd4afdaaa8357cd38ab Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 20 Jul 2026 20:13:14 -0400 Subject: [PATCH 02/13] refactor(config): simplify global config to delete access to typed object --- src/globalConfig/accessor.tsx | 123 +++------------------------- src/globalConfig/index.tsx | 7 +- src/globalConfig/source.tsx | 40 +++++---- src/globalConfig/types.tsx | 85 ++++++------------- src/handlers/config/config.test.tsx | 29 +------ src/handlers/config/handler.tsx | 61 +++++++++++--- 6 files changed, 115 insertions(+), 230 deletions(-) diff --git a/src/globalConfig/accessor.tsx b/src/globalConfig/accessor.tsx index 9e5685a79..8066b9ec3 100644 --- a/src/globalConfig/accessor.tsx +++ b/src/globalConfig/accessor.tsx @@ -1,134 +1,35 @@ -import z, { ZodError } from "zod"; -import { - globalConfigSchema, - type AsyncDataSource, - type GlobalConfigAccessor, - type GlobalConfigKey, - type GlobalConfigShape, -} from "./types"; -import { coerce, inspectZodSchema, formatZodError } from "../parsing"; +import { type GlobalConfig, type GlobalConfigAccessor, type JsonDataSource } from "./types"; import type { Logger } from "../logging"; -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - -function getAtPath(obj: object, path: string): unknown { - return path.split(".").reduce((acc, key) => (isRecord(acc) ? acc[key] : undefined), obj); -} - -function setAtPath( - obj: Record, - path: string, - value: unknown, -): Record { - const keys = path.split("."); - if (keys.length === 0) return obj; - const head = keys[0]!; - if (keys.length === 1) return { ...obj, [head]: value }; - return { - ...obj, - [head]: setAtPath( - isRecord(obj[head]) ? (obj[head] as Record) : {}, - keys.slice(1).join("."), - value, - ), - }; -} - -function buildKeys(schema: z.ZodObject, prefix: string = ""): string[] { - return Object.entries(schema.shape).flatMap(([key, value]) => { - const path = prefix ? `${prefix}.${key}` : key; - const unwrapped = inspectZodSchema(value as z.ZodType).innerType; - return unwrapped instanceof z.ZodObject ? [path, ...buildKeys(unwrapped, path)] : [path]; - }); -} - -function buildSchemas(schema: z.ZodObject, prefix: string = ""): Map { - return Object.entries(schema.shape).reduce((map, [key, value]) => { - const path = prefix ? `${prefix}.${key}` : key; - map.set(path, value as z.ZodType); - const unwrapped = inspectZodSchema(value as z.ZodType).innerType; - if (unwrapped instanceof z.ZodObject) { - for (const [k, v] of buildSchemas(unwrapped, path)) { - map.set(k, v); - } - } - return map; - }, new Map()); -} - export interface GlobalConfigAccessorConfig { logger: Logger; - source: AsyncDataSource; + source: JsonDataSource; } /** * Creates a {@link GlobalConfigAccessor} backed by the given data source. * - * The accessor provides typed get/put access to config values using dot-notation - * keys (e.g. `"telemetry.enabled"`), with schema validation and coercion on writes. - * - * @param config - The accessor configuration (file path and optional data source override). + * @param config - The logger and DataSource to be used by the config. . * @returns A {@link GlobalConfigAccessor} instance. */ export function createGlobalConfigAccessor( config: GlobalConfigAccessorConfig, ): GlobalConfigAccessor { - const validKeys = new Set(buildKeys(globalConfigSchema)); - const schemas = buildSchemas(globalConfigSchema); - config.logger.info(`creating global config accessor`); + let cachedConfig: GlobalConfig | undefined; const accessor: GlobalConfigAccessor = { - get: async (key) => { - config.logger.child({ key, method: "get" }).debug("reading value from global config"); - const data = await config.source.read(); - return getAtPath(data, key) as never; // cast since key is already typechecked. + get: async () => { + config.logger.child({ method: "get" }).debug("reading global config"); + return (cachedConfig ??= await config.source.read()); }, - put: async (key, value) => { - const logger = config.logger.child({ key, value, method: "put" }); - logger.debug("writing value to global config"); - - const data = await config.source.read(); - const schema = schemas.get(key); - - if (!schema) { - // TODO: make this a ValidationError (but mark it as a errorSource=client) - // note: this shouldn't happen unless we lie to the typesystem since key is strongly typed above, and map includes all schemas. - logger.error("unable to find schema associated with given key. The key is likely invalid"); - throw new Error( - `unable to put value '${JSON.stringify(value)}' for key '${key}' since key does not exist.`, - ); - } + set: async (newConfig) => { + const logger = config.logger.child({ newConfig, method: "set" }); + logger.debug("writing global config"); - try { - const coercedValue = coerce(schema, value); - await config.source.write(setAtPath(data, key, coercedValue)); - // note: cast is safe since write above will fail if not. - return coercedValue as typeof value; - } catch (e) { - const error = e instanceof Error ? e : new Error(String(e)); - logger - .child({ errorName: error.name, errorMessage: error.message }) - .error(`failed to write value to global config`); - - if (e instanceof ZodError) { - // TODO: swap to validation error. - throw new TypeError(formatZodError(e)); - } - throw e; - } - }, - - isValidKey: (key): key is GlobalConfigKey => { - config.logger.child({ key, method: "isValidKey" }).debug(); - return validKeys.has(key); - }, + cachedConfig = await config.source.write(newConfig); - toObject: () => { - config.logger.child({ method: "toObject" }).debug("retrieving entire config"); - return config.source.read(); + return cachedConfig; }, }; diff --git a/src/globalConfig/index.tsx b/src/globalConfig/index.tsx index c62ec1d35..52446fe52 100644 --- a/src/globalConfig/index.tsx +++ b/src/globalConfig/index.tsx @@ -1,8 +1,3 @@ -export { - type GlobalConfigAccessor, - type AsyncDataSource, - type GlobalConfigShape, - globalConfigSchema, -} from "./types"; +export { type GlobalConfigAccessor, type GlobalConfig, globalConfigSchema } from "./types"; export { createGlobalConfigAccessor } from "./accessor"; export { createJsonFileDataSource } from "./source"; diff --git a/src/globalConfig/source.tsx b/src/globalConfig/source.tsx index 01f0ba273..fd0ae7bb2 100644 --- a/src/globalConfig/source.tsx +++ b/src/globalConfig/source.tsx @@ -1,9 +1,9 @@ import z from "zod"; -import type { AsyncDataSource } from "./types"; import { readFile, writeFile, mkdir, rename, rm } from "fs/promises"; import { dirname } from "path"; import type { Logger } from "../logging"; -import { mapSchema } from "../parsing"; +import type { JsonDataSource } from "./types"; +import { formatZodError } from "../parsing"; export interface JsonFileDataSourceConfig { filePath: string; @@ -39,49 +39,57 @@ async function atomicWrite(filePath: string, data: string) { /** * Creates an {@link AsyncDataSource} backed by a JSON file on disk. * - * Reads are lenient (invalid fields fall back to `undefined`); writes are schema-strict. - * The file and parent directories are created automatically if missing. + * If the file does not exist, one will be created on first read with initialData or on write with the given data. * * @param config - File path, initial data, Zod schema, and logger. */ export function createJsonFileDataSource( config: JsonFileDataSourceConfig, -): AsyncDataSource> { +): JsonDataSource> { const logger = config.logger.child({ filePath: config.filePath }); logger.info(`creating data source from json file '${config.filePath}'`); - // note: we cast back to original schema type since adding fallback shouldn't change the produced type. - // using a lenient schema here allows us to read "partially" bad configs and ignore bad entries. - const readSchema = mapSchema(config.schema, (s) => s.catch(undefined)) as typeof config.schema; - - return { + const source: JsonDataSource> = { read: async () => { try { const raw = await readFile(config.filePath, "utf-8"); - return readSchema.parse(JSON.parse(raw)); + return config.schema.parse(JSON.parse(raw)); } catch (e) { // If the file doesn't exist, create one for them with the defaults. if (isNodeError(e) && e.code === "ENOENT") { logger.warn(`unable to find json file, creating default`); - await mkdir(dirname(config.filePath), { recursive: true }); - await atomicWrite(config.filePath, JSON.stringify(config.initialData, null, 2)); - return config.initialData; + return source.write(config.initialData); } - const error = e instanceof Error ? e : new Error(String(e)); logger .child({ errorName: error.name, errorMessage: error.message }) .error("failed to read config file"); + // TODO: swap to typed error. throw new Error(`unable to read json file at '${config.filePath}'`); } }, write: async (data) => { - const parsedData = config.schema.parse(data); + const parseDataResult = config.schema.safeParse(data); + + if (!parseDataResult.success) { + logger + .child({ + errorName: parseDataResult.error.name, + errorMessage: parseDataResult.error.message, + }) + .error(`failed to validate data on write`); + // TODO: swap to validation. + throw new TypeError(formatZodError(parseDataResult.error)); + } + + const parsedData = parseDataResult.data; await mkdir(dirname(config.filePath), { recursive: true }); await atomicWrite(config.filePath, JSON.stringify(parsedData, null, 2)); return parsedData; }, }; + + return source; } diff --git a/src/globalConfig/types.tsx b/src/globalConfig/types.tsx index 49ee899c6..51c96bff1 100644 --- a/src/globalConfig/types.tsx +++ b/src/globalConfig/types.tsx @@ -1,78 +1,47 @@ import z from "zod"; -/** - * Builds a union type of all fields and their respective shapes recursively via dot notation. - * Ex. - * { user: { name: string, age: number }} -> { user: { name: string, age: number }} | { "user.name": string } | { "user.age": number } - */ -type ObjectEntryShapes = { - [ - TKey in keyof TObject & string // for each key of the object. - ]: - | { [_ in `${TPrefix}${TKey}`]: TObject[TKey] } // extract out the name of each key - | (NonNullable extends Record // check if the value at the key is an object itself. - ? ObjectEntryShapes, `${TPrefix}${TKey}.`> // if it is recurse - : never); // otherwise we are done -}[keyof TObject & string]; // access values to unnest - -/** - * Given a union of object types, returns a object type describing their intersection. - * Ex. - * { user: { name: string, age: number }} | { "user.name": string } | { "user.age": number } - * -> { user: { name: string, age: number }, "user.name": string, "user.age": number } - */ -type UnionToIntersection = (TUnion extends any ? (x: TUnion) => void : never) extends ( - x: infer I, -) => void - ? I - : never; - -type FlattenObject = UnionToIntersection>; - -/** - * A source of data that follows the given shape. - */ -export interface AsyncDataSource { - read(): Promise; - write(s: unknown): Promise; -} - +const coeredBoolean = z.preprocess((val) => { + if (typeof val === "string") { + if (val === "true" || val === "1") return true; + if (val === "false" || val === "0") return false; + } + return val; +}, z.boolean()); + +const coerceObject = (shape: TShape) => + z.preprocess((val) => { + if (typeof val === "string") { + return JSON.parse(val); + } + return val; + }, z.object(shape)); /** * Schema for the global config file. {@link GlobalConfigAccessor} is built from this definition. * Note: all fields should be optional, missing fields in global config should be overriden with reasonable defaults in the consuming code. */ export const globalConfigSchema = z.object({ - telemetry: z - .object({ - enabled: z.boolean().optional(), - endpoint: z.string().optional(), - }) - .optional(), + telemetry: coerceObject({ + enabled: coeredBoolean.optional(), + endpoint: z.string().optional(), + }).optional(), installationId: z.uuid().optional(), }); /** * Shape of the global config built from {@link globalConfigSchema} */ -export type GlobalConfigShape = z.infer; +export type GlobalConfig = z.infer; -/** - * Flattened global config shape that leverages dot notation instead of nested objects. - */ -type FlattenedGlobalConfigShape = FlattenObject; - -export type GlobalConfigKey = keyof FlattenedGlobalConfigShape & string; -export type GlobalConfigValue = FlattenedGlobalConfigShape[TKey]; /** * Interface for setting and retrieving information from the global configuration for the CLI. * ex. telemetry settings. */ export interface GlobalConfigAccessor { - get(key: TKey): Promise | undefined>; - put( - key: TKey, - value: GlobalConfigValue, - ): Promise>; - isValidKey(key: string): key is GlobalConfigKey; - toObject(): Promise; + get(): Promise; + set(newConfig: GlobalConfig): Promise; +} + +export interface JsonDataSource { + read(): Promise; + write(data: TShape): Promise; } diff --git a/src/handlers/config/config.test.tsx b/src/handlers/config/config.test.tsx index 0ac595621..8dbd7ae2e 100644 --- a/src/handlers/config/config.test.tsx +++ b/src/handlers/config/config.test.tsx @@ -122,39 +122,14 @@ describe("config", () => { expect(JSON.parse(readEndpointOutput)).toBe("false"); }); - test("tolerant of partially invalid config files", async () => { - const invalidConfig = { - ...validConfig, - installationId: "purple", - }; - await writeFile(configPath, JSON.stringify(invalidConfig)); - - const fullReadOutput = await run([]); - - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { installationId, ...rest } = invalidConfig; - expect(JSON.parse(fullReadOutput)).toEqual(rest); - - const partialReadout = await run(["installationId"]); - expect(partialReadout).toEqual(""); - - const uuid = crypto.randomUUID(); - const writeOutput = await run(["installationId", uuid]); - expect(JSON.parse(writeOutput)).toBe(uuid); - - const readOutput = await run(["installationId"]); - expect(readOutput).toEqual(`"${uuid}"`); - }); - - test("tolerates a single bad field inside a nested object without losing sibling fields", async () => { + test("throws on a single bad field", async () => { await writeFile( configPath, JSON.stringify({ telemetry: { enabled: "not-a-bool", endpoint: "https://good.com" }, }), ); - const output = await run(["telemetry.endpoint"]); - expect(JSON.parse(output)).toBe("https://good.com"); + await expect(async () => run(["telemetry.endpoint"])).toThrow("unable to read"); }); test("forward compatible with unsupported fields", async () => { diff --git a/src/handlers/config/handler.tsx b/src/handlers/config/handler.tsx index 856977f37..9b8f6d5c7 100644 --- a/src/handlers/config/handler.tsx +++ b/src/handlers/config/handler.tsx @@ -2,9 +2,10 @@ import z from "zod"; import { createHandler, argument } from "../../router"; import { GlobalConfigKey } from "../../middleware"; import { JsonRendererKey } from "../../tui"; +import { globalConfigSchema, type GlobalConfig } from "../../globalConfig"; /* - * read/write global configuration values. ex. telemetry settings, log level, etc. + * read/write global configuration values. ex. telemetry settings, endpoint overrides, etc. * Ex. * `config [key] [value]` sets key to value. * `config [key]` prints the value with key. @@ -26,27 +27,63 @@ export const createConfigHandler = () => const globalConfigAccessor = ctx.require(GlobalConfigKey); const jsonRenderer = ctx.require(JsonRendererKey); + const globalConfig = await globalConfigAccessor.get(); // print entire config when key is missing. if (!args.key) { - const fullConfig = await globalConfigAccessor.toObject(); - if (fullConfig !== undefined) jsonRenderer.renderJson(fullConfig); + if (globalConfig !== undefined) jsonRenderer.renderJson(globalConfig); return; } - const configKey = args.key; - if (!globalConfigAccessor.isValidKey(configKey)) { - // TODO: make this a validation error. - throw new TypeError(`config key '${configKey}' does not exist.`); - } - // print entire value at key when value is missing. if (!args.value) { - const scopedConfig = await globalConfigAccessor.get(configKey); + const scopedConfig = getAtPath(globalConfig, args.key); + if (scopedConfig === undefined && !isValidPath(globalConfig, args.key)) + throw new TypeError(`invalid key ${args.key} for global config`); if (scopedConfig !== undefined) jsonRenderer.renderJson(scopedConfig); return; } // update value at key with given value. - const newValue = await globalConfigAccessor.put(configKey, args.value); - jsonRenderer.renderJson(newValue); + const rawUpdatedConfig = setAtPath(globalConfig, args.key, args.value); + const updatedConfig = await globalConfigAccessor.set(rawUpdatedConfig); + const updatedScopedConfig = getAtPath(updatedConfig, args.key); + + if (updatedScopedConfig === undefined && !isValidPath(updatedConfig, args.key)) + throw new TypeError(`invalid key ${args.key} for global config`); + jsonRenderer.renderJson(updatedScopedConfig); }, }); + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function getAtPath(obj: object, path: string): unknown { + return path.split(".").reduce((acc, key) => (isRecord(acc) ? acc[key] : undefined), obj); +} + +function setAtPath( + obj: Record, + path: string, + value: unknown, +): Record { + const keys = path.split("."); + if (keys.length === 0) return obj; + const head = keys[0]!; + if (keys.length === 1) return { ...obj, [head]: value }; + return { + ...obj, + [head]: setAtPath( + isRecord(obj[head]) ? (obj[head] as Record) : {}, + keys.slice(1).join("."), + value, + ), + }; +} + +function isValidPath(config: GlobalConfig, path: string): boolean { + const parseAttempt = globalConfigSchema.safeParse(setAtPath(config, path, "placeholder")); + // if parse fails --> zod validated the key so it must be known. + if (!parseAttempt.success) return true; + // if parse passes -> zod drops the key implies its not known, but including it means its valid and has type string. + return getAtPath(parseAttempt.data, path) !== undefined; +} From 30cfeed5c33ac23ed7839f3ce05ed07ecc87b59e Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 20 Jul 2026 20:20:22 -0400 Subject: [PATCH 03/13] refactor(parsing): move parsing back down into router --- src/globalConfig/accessor.tsx | 12 +- src/globalConfig/index.tsx | 3 +- src/globalConfig/schemas.tsx | 110 +++++++++++++ src/globalConfig/source.tsx | 11 +- src/globalConfig/types.tsx | 48 ++---- src/handlers/config/config.test.tsx | 30 +++- src/handlers/config/handler.tsx | 22 +-- src/handlers/index.tsx | 4 +- src/index.ts | 2 +- src/middleware/index.tsx | 2 +- ...onfig.tsx => withGlobalConfigAccessor.tsx} | 12 +- src/router/args.tsx | 4 +- src/router/flags.tsx | 4 +- src/router/index.tsx | 1 + src/router/router.tsx | 4 + src/{parsing/index.tsx => router/schema.tsx} | 150 ++++++------------ src/testing/globalConfig.tsx | 6 +- 17 files changed, 233 insertions(+), 192 deletions(-) create mode 100644 src/globalConfig/schemas.tsx rename src/middleware/{withGlobalConfig.tsx => withGlobalConfigAccessor.tsx} (51%) rename src/{parsing/index.tsx => router/schema.tsx} (52%) diff --git a/src/globalConfig/accessor.tsx b/src/globalConfig/accessor.tsx index 8066b9ec3..0ad0b3a9d 100644 --- a/src/globalConfig/accessor.tsx +++ b/src/globalConfig/accessor.tsx @@ -19,16 +19,18 @@ export function createGlobalConfigAccessor( let cachedConfig: GlobalConfig | undefined; const accessor: GlobalConfigAccessor = { get: async () => { - config.logger.child({ method: "get" }).debug("reading global config"); - return (cachedConfig ??= await config.source.read()); + const logger = config.logger.child({ method: "get" }); + logger.debug("reading global config"); + if (cachedConfig) return cachedConfig; + logger.debug("failed to find cache, reading from source"); + cachedConfig = await config.source.read(); + return cachedConfig; }, set: async (newConfig) => { - const logger = config.logger.child({ newConfig, method: "set" }); - logger.debug("writing global config"); + config.logger.child({ newConfig, method: "set" }).debug("writing global config"); cachedConfig = await config.source.write(newConfig); - return cachedConfig; }, }; diff --git a/src/globalConfig/index.tsx b/src/globalConfig/index.tsx index 52446fe52..a66aeaac0 100644 --- a/src/globalConfig/index.tsx +++ b/src/globalConfig/index.tsx @@ -1,3 +1,4 @@ -export { type GlobalConfigAccessor, type GlobalConfig, globalConfigSchema } from "./types"; +export { type GlobalConfigAccessor, type GlobalConfig, type JsonDataSource } from "./types"; export { createGlobalConfigAccessor } from "./accessor"; export { createJsonFileDataSource } from "./source"; +export { globalConfigSchema, isValidKey } from "./schemas"; diff --git a/src/globalConfig/schemas.tsx b/src/globalConfig/schemas.tsx new file mode 100644 index 000000000..3accbf148 --- /dev/null +++ b/src/globalConfig/schemas.tsx @@ -0,0 +1,110 @@ +import z from "zod"; + +/** Zod boolean schema that coerces string values (`"true"`, `"1"`, `"false"`, `"0"`) to booleans. */ +const coercedBoolean = z.preprocess((val) => { + if (typeof val === "string") { + if (val === "true" || val === "1") return true; + if (val === "false" || val === "0") return false; + } + return val; +}, z.boolean()); + +/** Creates a Zod object schema that coerces JSON strings into objects before validation. */ +const coerceObject = (shape: TShape) => + z.preprocess((val) => { + if (typeof val === "string") { + try { + return JSON.parse(val); + } catch { + return val; + } + } + return val; + }, z.object(shape)); + +/** + * Schema for the global config file. {@link GlobalConfigAccessor} is built from this definition. + * All fields should be optional; missing fields should be overridden with reasonable defaults in the consuming code. + */ +export const globalConfigSchema = z.object({ + telemetry: coerceObject({ + enabled: coercedBoolean.optional(), + endpoint: z.string().optional(), + audit: coercedBoolean.optional(), + }).optional(), + installationId: z.uuid().optional(), +}); + +/** Executes `fn` on first call and caches the result for subsequent calls. */ +function once(fn: () => T): () => T { + let result: T; + let called = false; + return () => { + if (!called) { + result = fn(); + called = true; + } + return result; + }; +} + +/** + * Returns the internal def for a zod schema, which contains the `type` discriminant + * and structural fields like `innerType`, `shape`, `out`, etc. + * @see https://zod.dev/packages/core + */ +function getDef(schema: z.ZodType): { + type: string; + innerType?: z.ZodType; + out?: z.ZodType; + shape?: Record; +} { + return (schema as any)._zod.def; +} + +/** Peels wrapper types to reach the base schema. */ +function unwrapSchema(schema: z.ZodType): z.ZodType { + for (;;) { + const def = getDef(schema); + switch (def.type) { + case "optional": + case "nullable": + case "default": + case "readonly": + schema = def.innerType!; + break; + case "pipe": + // z.preprocess() returns a ZodPipe — take the output schema + schema = def.out!; + break; + default: + return schema; + } + } +} + +/** Recursively collects all valid dot-notation paths from a ZodObject schema. */ +function buildKeySet(schema: z.ZodObject, prefix = ""): Set { + const paths = new Set(); + + for (const key of schema.keyof().options) { + const fullPath = prefix ? `${prefix}.${key}` : key; + paths.add(fullPath); + + const inner = unwrapSchema(schema.shape[key]); + if (getDef(inner).type === "object") { + for (const nested of buildKeySet(inner as z.ZodObject, fullPath)) { + paths.add(nested); + } + } + } + + return paths; +} + +const getValidKeys = once(() => buildKeySet(globalConfigSchema)); + +/** Returns whether a dot-notation path is a known key in {@link globalConfigSchema}. */ +export function isValidKey(path: string): boolean { + return getValidKeys().has(path); +} diff --git a/src/globalConfig/source.tsx b/src/globalConfig/source.tsx index fd0ae7bb2..27a29f871 100644 --- a/src/globalConfig/source.tsx +++ b/src/globalConfig/source.tsx @@ -3,14 +3,13 @@ import { readFile, writeFile, mkdir, rename, rm } from "fs/promises"; import { dirname } from "path"; import type { Logger } from "../logging"; import type { JsonDataSource } from "./types"; -import { formatZodError } from "../parsing"; -export interface JsonFileDataSourceConfig { +export interface JsonFileDataSourceConfig { filePath: string; /** * Describes the data to write to the file if it does not exist or is deleted. */ - initialData: z.infer; + getDefaultData(): z.infer; schema: TSchema; logger: Logger; } @@ -55,10 +54,10 @@ export function createJsonFileDataSource( const raw = await readFile(config.filePath, "utf-8"); return config.schema.parse(JSON.parse(raw)); } catch (e) { - // If the file doesn't exist, create one for them with the defaults. + // If the file doesn't exist, create one with the defaults. if (isNodeError(e) && e.code === "ENOENT") { logger.warn(`unable to find json file, creating default`); - return source.write(config.initialData); + return await source.write(config.getDefaultData()); } const error = e instanceof Error ? e : new Error(String(e)); @@ -81,7 +80,7 @@ export function createJsonFileDataSource( }) .error(`failed to validate data on write`); // TODO: swap to validation. - throw new TypeError(formatZodError(parseDataResult.error)); + throw new TypeError(z.prettifyError(parseDataResult.error)); } const parsedData = parseDataResult.data; diff --git a/src/globalConfig/types.tsx b/src/globalConfig/types.tsx index 51c96bff1..03efba31a 100644 --- a/src/globalConfig/types.tsx +++ b/src/globalConfig/types.tsx @@ -1,47 +1,21 @@ -import z from "zod"; +import type z from "zod"; +import type { globalConfigSchema } from "./schemas"; -const coeredBoolean = z.preprocess((val) => { - if (typeof val === "string") { - if (val === "true" || val === "1") return true; - if (val === "false" || val === "0") return false; - } - return val; -}, z.boolean()); - -const coerceObject = (shape: TShape) => - z.preprocess((val) => { - if (typeof val === "string") { - return JSON.parse(val); - } - return val; - }, z.object(shape)); -/** - * Schema for the global config file. {@link GlobalConfigAccessor} is built from this definition. - * Note: all fields should be optional, missing fields in global config should be overriden with reasonable defaults in the consuming code. - */ -export const globalConfigSchema = z.object({ - telemetry: coerceObject({ - enabled: coeredBoolean.optional(), - endpoint: z.string().optional(), - }).optional(), - installationId: z.uuid().optional(), -}); - -/** - * Shape of the global config built from {@link globalConfigSchema} - */ +/** Inferred shape of the global config from {@link globalConfigSchema}. */ export type GlobalConfig = z.infer; -/** - * Interface for setting and retrieving information from the global configuration for the CLI. - * ex. telemetry settings. - */ +/** Reads and writes global CLI configuration. */ export interface GlobalConfigAccessor { + /** Returns the current global config. */ get(): Promise; - set(newConfig: GlobalConfig): Promise; + /** Validates and persists a new config. Throws on invalid shape. */ + set(newConfig: unknown): Promise; } +/** Generic read/write source for a typed JSON object. */ export interface JsonDataSource { + /** Reads data from the source. */ read(): Promise; - write(data: TShape): Promise; + /** Validates and writes data to the source. Throws on invalid shape. */ + write(data: unknown): Promise; } diff --git a/src/handlers/config/config.test.tsx b/src/handlers/config/config.test.tsx index 8dbd7ae2e..2ad810866 100644 --- a/src/handlers/config/config.test.tsx +++ b/src/handlers/config/config.test.tsx @@ -1,6 +1,6 @@ import { test, expect, describe, beforeEach, afterEach } from "bun:test"; import { join } from "node:path"; -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, rm, writeFile, readFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { createRootHandler } from "../index"; import { createSilentLogger, TestCoreClient, testIO } from "../../testing"; @@ -41,7 +41,7 @@ describe("config", () => { source: createJsonFileDataSource({ filePath: configPath, schema: globalConfigSchema, - initialData: initialConfig, + getDefaultData: () => initialConfig, logger, }), }); @@ -80,6 +80,12 @@ describe("config", () => { const readValueOutput = await run(["telemetry.endpoint"]); expect(JSON.parse(readValueOutput)).toBe(newEndpoint); + + // validate the new value persists on disk. + expect(JSON.parse(await readFile(configPath, "utf8"))).toMatchObject({ + ...validConfig, + telemetry: { ...validConfig.telemetry, endpoint: newEndpoint }, + }); }); test("accepts json values for non-leaf nodes", async () => { @@ -102,12 +108,17 @@ describe("config", () => { test("throws on invalid key", async () => { // TODO: swap to validation error. - expect(run(["nonexistent.key"])).rejects.toThrow(TypeError); + await expect(run(["nonexistent.key"])).rejects.toThrow(TypeError); + }); + + test("does not throw on valid, but non-existent key", async () => { + const output = await run(["telemetry.audit"]); + expect(output).toBe(""); }); test("throws on invalid value for key", async () => { // TODO: swap to validation error - expect(run(["telemetry.enabled", "banana"])).rejects.toThrow(TypeError); + await expect(run(["telemetry.enabled", "banana"])).rejects.toThrow(TypeError); }); test("coerces values based on schema", async () => { @@ -122,17 +133,17 @@ describe("config", () => { expect(JSON.parse(readEndpointOutput)).toBe("false"); }); - test("throws on a single bad field", async () => { + test("throws if any field fails validation", async () => { await writeFile( configPath, JSON.stringify({ telemetry: { enabled: "not-a-bool", endpoint: "https://good.com" }, }), ); - await expect(async () => run(["telemetry.endpoint"])).toThrow("unable to read"); + await expect(run(["telemetry.endpoint"])).rejects.toThrow("unable to read"); }); - test("forward compatible with unsupported fields", async () => { + test("ignores unsupported fields in the config file", async () => { await writeFile( configPath, JSON.stringify({ @@ -148,7 +159,7 @@ describe("config", () => { test("throws clear error on invalid json", async () => { await writeFile(configPath, "not { valid json"); // TODO: assert on error type - expect(run([])).rejects.toThrow("unable to read"); + await expect(run([])).rejects.toThrow("unable to read"); }); test("creates default config if it does not exist", async () => { @@ -160,5 +171,8 @@ describe("config", () => { const readOutput = await run(["telemetry.endpoint"]); expect(JSON.parse(readOutput)).toBe(newEndpoint); + + const installationIdReadOutput = await run(["installationId"]); + expect(JSON.parse(installationIdReadOutput)).toBe(initialConfig.installationId); }); }); diff --git a/src/handlers/config/handler.tsx b/src/handlers/config/handler.tsx index 9b8f6d5c7..8785a3842 100644 --- a/src/handlers/config/handler.tsx +++ b/src/handlers/config/handler.tsx @@ -1,8 +1,7 @@ import z from "zod"; -import { createHandler, argument } from "../../router"; -import { GlobalConfigKey } from "../../middleware"; +import { createHandler, argument, GlobalConfigAccessorKey } from "../../router"; import { JsonRendererKey } from "../../tui"; -import { globalConfigSchema, type GlobalConfig } from "../../globalConfig"; +import { isValidKey } from "../../globalConfig"; /* * read/write global configuration values. ex. telemetry settings, endpoint overrides, etc. @@ -24,7 +23,7 @@ export const createConfigHandler = () => argument("value", "value to set for the key", z.string().optional()), ], handle: async (ctx, _flags, args) => { - const globalConfigAccessor = ctx.require(GlobalConfigKey); + const globalConfigAccessor = ctx.require(GlobalConfigAccessorKey); const jsonRenderer = ctx.require(JsonRendererKey); const globalConfig = await globalConfigAccessor.get(); @@ -37,7 +36,7 @@ export const createConfigHandler = () => // print entire value at key when value is missing. if (!args.value) { const scopedConfig = getAtPath(globalConfig, args.key); - if (scopedConfig === undefined && !isValidPath(globalConfig, args.key)) + if (scopedConfig === undefined && !isValidKey(args.key)) throw new TypeError(`invalid key ${args.key} for global config`); if (scopedConfig !== undefined) jsonRenderer.renderJson(scopedConfig); return; @@ -47,20 +46,21 @@ export const createConfigHandler = () => const updatedConfig = await globalConfigAccessor.set(rawUpdatedConfig); const updatedScopedConfig = getAtPath(updatedConfig, args.key); - if (updatedScopedConfig === undefined && !isValidPath(updatedConfig, args.key)) - throw new TypeError(`invalid key ${args.key} for global config`); jsonRenderer.renderJson(updatedScopedConfig); }, }); +/** Type guard that narrows `value` to a plain object record. */ function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } +/** Retrieves a nested value from `obj` using dot-notation (e.g. `"telemetry.enabled"`). */ function getAtPath(obj: object, path: string): unknown { return path.split(".").reduce((acc, key) => (isRecord(acc) ? acc[key] : undefined), obj); } +/** Returns a shallow-cloned object with `value` set at the given dot-notation `path`. */ function setAtPath( obj: Record, path: string, @@ -79,11 +79,3 @@ function setAtPath( ), }; } - -function isValidPath(config: GlobalConfig, path: string): boolean { - const parseAttempt = globalConfigSchema.safeParse(setAtPath(config, path, "placeholder")); - // if parse fails --> zod validated the key so it must be known. - if (!parseAttempt.success) return true; - // if parse passes -> zod drops the key implies its not known, but including it means its valid and has type string. - return getAtPath(parseAttempt.data, path) !== undefined; -} diff --git a/src/handlers/index.tsx b/src/handlers/index.tsx index b0a26e187..a86bdf2cf 100644 --- a/src/handlers/index.tsx +++ b/src/handlers/index.tsx @@ -5,7 +5,7 @@ import { DebugKey, EndpointKey, JsonKey, RegionKey } from "./keys.tsx"; import { createConfigHandler } from "./config/"; import { createProjectHandler } from "./project/index.ts"; import { renderTui } from "../tui"; -import { withRegion, withJsonRenderer, withLogging, withGlobalConfig } from "../middleware"; +import { withRegion, withJsonRenderer, withLogging, withGlobalConfigAccessor } from "../middleware"; import type { AppIO, Core } from "./types.tsx"; import type { Logger } from "../logging"; import type { GlobalConfigAccessor } from "../globalConfig"; @@ -35,7 +35,7 @@ export function createRootHandler(core: Core, config: RootHandlerConfig): Router root.use(withLogging({ logger })); // Pin the global config accessor on the context for any handler that needs it. - root.use(withGlobalConfig(config.globalConfigAccessor)); + root.use(withGlobalConfigAccessor(config.globalConfigAccessor)); // Install sub handlers root.handler(createHarnessHandler(core, io)); diff --git a/src/index.ts b/src/index.ts index eaf80d42d..49dda2d29 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,7 +42,7 @@ process.exit( source: createJsonFileDataSource({ filePath: join(homedir(), ".agentcore", "config.json"), schema: globalConfigSchema, - initialData: { installationId: crypto.randomUUID() }, + getDefaultData: () => ({ installationId: crypto.randomUUID() }), logger: rootLogger.child({ module: "jsonDataSource" }), }), }); diff --git a/src/middleware/index.tsx b/src/middleware/index.tsx index ac3ab04d8..e77355898 100644 --- a/src/middleware/index.tsx +++ b/src/middleware/index.tsx @@ -2,4 +2,4 @@ export { withRegion } from "./withRegion"; export { withTuiOnEmptyFlagsAndArgs } from "./withTuiOnEmptyFlagsAndArgs"; export { withJsonRenderer } from "./withJsonRenderer"; export { withLogging } from "./withLogging"; -export { withGlobalConfig, GlobalConfigKey } from "./withGlobalConfig"; +export { withGlobalConfigAccessor } from "./withGlobalConfigAccessor"; diff --git a/src/middleware/withGlobalConfig.tsx b/src/middleware/withGlobalConfigAccessor.tsx similarity index 51% rename from src/middleware/withGlobalConfig.tsx rename to src/middleware/withGlobalConfigAccessor.tsx index 4c47e4904..8acd2b056 100644 --- a/src/middleware/withGlobalConfig.tsx +++ b/src/middleware/withGlobalConfigAccessor.tsx @@ -1,16 +1,12 @@ -import type { Middleware } from "../router"; -import { contextKey, type ContextKey } from "../router"; +import { GlobalConfigAccessorKey, type Middleware } from "../router"; import type { GlobalConfigAccessor } from "../globalConfig"; -export const GlobalConfigKey: ContextKey = - contextKey("globalConfig"); - /** * Middleware that pins a GlobalConfigAccessor on the context, making it * available to every handler beneath the mount point via - * `ctx.require(GlobalConfigKey)`. + * `ctx.require(GlobalConfigAccessorKey)`. */ -export function withGlobalConfig(accessor: GlobalConfigAccessor): Middleware { +export function withGlobalConfigAccessor(accessor: GlobalConfigAccessor): Middleware { return (h) => ({ name: () => h.name(), description: () => h.description(), @@ -18,7 +14,7 @@ export function withGlobalConfig(accessor: GlobalConfigAccessor): Middleware { arguments: () => h.arguments(), children: () => h.children(), handle: async (ctx, flags, args) => { - await h.handle(ctx.withValue(GlobalConfigKey, accessor), flags, args); + await h.handle(ctx.withValue(GlobalConfigAccessorKey, accessor), flags, args); }, }); } diff --git a/src/router/args.tsx b/src/router/args.tsx index f376d2ea4..dd8693eb1 100644 --- a/src/router/args.tsx +++ b/src/router/args.tsx @@ -1,9 +1,9 @@ import { type Command, Argument as CommanderArgument } from "commander"; import type { Argument } from "./handler"; -import { coerce, formatZodError, inspectZodSchema } from "../parsing"; +import { coerce, formatZodError, inspect } from "./schema"; export function toCommanderArgument(arg: Argument): CommanderArgument { - const info = inspectZodSchema(arg.schema); + const info = inspect(arg.schema); // Commander treats <> as required and [] as optional: https://github.com/tj/commander.js#more-configuration-1 // Variadic arguments use trailing ... syntax: https://github.com/tj/commander.js#command-arguments const inner = info.variadic ? `${arg.name}...` : arg.name; diff --git a/src/router/flags.tsx b/src/router/flags.tsx index 07cb5ba6e..07fb55c91 100644 --- a/src/router/flags.tsx +++ b/src/router/flags.tsx @@ -1,13 +1,13 @@ import { Option } from "commander"; import type { Context } from "./context"; import type { Flag, GlobalFlag } from "./handler"; -import { coerce, formatZodError, inspectZodSchema } from "../parsing"; +import { coerce, formatZodError, inspect } from "./schema"; // toOption builds a Commander Option from a flag's schema. Booleans become value-less // toggles; everything else takes a value (`` / variadic ``). A // required, non-boolean flag is made mandatory; defaults are forwarded. export function toOption(flag: Flag): Option { - const info = inspectZodSchema(flag.schema); + const info = inspect(flag.schema); const long = `--${flag.name}`; let token: string; diff --git a/src/router/index.tsx b/src/router/index.tsx index 98f581fa8..0b2d6c1cc 100644 --- a/src/router/index.tsx +++ b/src/router/index.tsx @@ -4,6 +4,7 @@ export { CommandKey, PathKey, LoggerKey, + GlobalConfigAccessorKey, type DefaultHandle, type DefaultHandlerProvider, isDefaultHandlerProvider, diff --git a/src/router/router.tsx b/src/router/router.tsx index 5aec62670..583113ea3 100644 --- a/src/router/router.tsx +++ b/src/router/router.tsx @@ -6,6 +6,7 @@ import { parseArguments, toCommanderArgument } from "./args"; import { Command } from "commander"; import type { Logger } from "../logging"; +import type { GlobalConfigAccessor } from "../globalConfig"; // CommandKey exposes the Commander Command for the executing leaf via context. export const CommandKey: ContextKey = contextKey("commander.command"); @@ -14,6 +15,9 @@ export const PathKey: ContextKey = contextKey("path"); export const LoggerKey = contextKey("logger"); +export const GlobalConfigAccessorKey: ContextKey = + contextKey("globalConfigAccessor"); + // DefaultHandle runs when a group is selected without a subcommand (e.g. // `agentcore` or `agentcore harness`). It reads group-level/global flags from the // context; own flags/arguments are not supported, so it receives empty objects. diff --git a/src/parsing/index.tsx b/src/router/schema.tsx similarity index 52% rename from src/parsing/index.tsx rename to src/router/schema.tsx index eda22ea2a..d89608457 100644 --- a/src/parsing/index.tsx +++ b/src/router/schema.tsx @@ -1,51 +1,14 @@ -import type { ZodType } from "zod"; -import z from "zod"; +import type z from "zod"; -export function formatZodError(error: z.ZodError): string { - return error.issues.map((issue) => issue.message).join("; "); -} - -/** - * Converts a raw value into the type the schema expects. - * - * @param schema - The zod schema describing the expected type. - * @param raw - The raw value - * @returns the coerced value matching the schema's expected type or the original value if unable to coerce - */ -export function coerce(schema: z.ZodType, raw: unknown): unknown { - if (raw === undefined) return raw; // defer to schema default / optionality - if (typeof raw === "boolean") return raw; // boolean toggles are already parsed - const type = baseType(schema); - if (Array.isArray(raw)) { - return raw.map((r) => coerceScalar(type, String(r))); - } - if (type === "object") { - return JSON.parse(String(raw)); - } - return coerceScalar(type, String(raw)); -} - -function coerceScalar(type: string | undefined, raw: string): unknown { - switch (type) { - case "number": - return Number(raw); - case "bigint": - try { - return BigInt(raw); - } catch { - return raw; // let zod produce the validation error - } - case "boolean": { - const v = raw.toLowerCase(); - if (v === "true" || v === "1" || v === "yes") return true; - if (v === "false" || v === "0" || v === "no") return false; - return raw; - } - case "date": - return new Date(raw); - default: - return raw; - } +// Inspection describes how a zod schema maps onto a Commander option: whether a +// value must be supplied (required), whether it is variadic (an array), whether +// it is a boolean toggle, and any default value. +interface Inspection { + required: boolean; + variadic: boolean; + boolean: boolean; + hasDefault: boolean; + defaultValue: unknown; } // zod v4 exposes a `.def` with a `type` discriminant and wrapper-specific fields @@ -69,26 +32,9 @@ function baseType(schema: z.ZodType): string | undefined { } } -/** - * Describes attributes of a zod schema, such as if its required, hasDefaults, and its innerType. - */ -export interface ZodSchemaInspection { - required: boolean; - variadic: boolean; - boolean: boolean; - hasDefault: boolean; - defaultValue: unknown; - innerType: z.ZodType; -} - -/** - * Peels optional/default/nullable/readonly wrappers off a schema to determine - * the option shape, any default, and the underlying inner type. - * - * @param schema - The zod schema to inspect. - * @returns An inspection describing the schema's structure. - */ -export function inspectZodSchema(schema: z.ZodType): ZodSchemaInspection { +// inspect peels optional/default/nullable/readonly wrappers off a schema to +// determine the option shape and any default. +export function inspect(schema: z.ZodType): Inspection { const required = !schema.isOptional(); let hasDefault = false; let defaultValue: unknown = undefined; @@ -111,42 +57,44 @@ export function inspectZodSchema(schema: z.ZodType): ZodSchemaInspection { } } - return { - required, - variadic, - boolean: baseType(schema) === "boolean", - hasDefault, - defaultValue, - innerType: s as ZodType, - }; + return { required, variadic, boolean: baseType(schema) === "boolean", hasDefault, defaultValue }; } -/** - * Recursively applies `fn` to every leaf schema in a zod object tree, preserving - * wrapper types (optional, nullable, default, readonly) on the way back up. - * - * @param schema - The zod schema to traverse. - * @param fn - Transformation applied to each leaf (and each object after its shape is mapped). - */ -export function mapSchema(schema: z.ZodType, fn: (s: z.ZodType) => z.ZodType): z.ZodType { - const s = schema as unknown as AnyDef; - const t = s.def?.type; - if (t === "object" && "shape" in schema) { - const obj = schema as z.ZodObject; - const newShape: Record = {}; - for (const [key, value] of Object.entries(obj.shape)) { - newShape[key] = mapSchema(value as z.ZodType, fn); +function coerceScalar(type: string | undefined, raw: string): unknown { + switch (type) { + case "number": + return Number(raw); + case "bigint": + try { + return BigInt(raw); + } catch { + return raw; // let zod produce the validation error + } + case "boolean": { + const v = raw.toLowerCase(); + if (v === "true" || v === "1" || v === "yes") return true; + if (v === "false" || v === "0" || v === "no") return false; + return raw; } - return fn(z.object(newShape)); - } else if (t === "optional") { - return mapSchema(s.def!.innerType as unknown as z.ZodType, fn).optional(); - } else if (t === "nullable") { - return mapSchema(s.def!.innerType as unknown as z.ZodType, fn).nullable(); - } else if (t === "default") { - return mapSchema(s.def!.innerType as unknown as z.ZodType, fn).default(s.def!.defaultValue); - } else if (t === "readonly") { - return mapSchema(s.def!.innerType as unknown as z.ZodType, fn).readonly(); - } else { - return fn(schema); + case "date": + return new Date(raw); + default: + return raw; + } +} + +// coerce converts the raw Commander value (a string, string[] for variadic +// options, or a boolean for toggles) into the type the schema expects. +export function coerce(schema: z.ZodType, raw: unknown): unknown { + if (raw === undefined) return raw; // defer to schema default / optionality + if (typeof raw === "boolean") return raw; // boolean toggles are already parsed + const type = baseType(schema); + if (Array.isArray(raw)) { + return raw.map((r) => coerceScalar(type, String(r))); } + return coerceScalar(type, String(raw)); +} + +export function formatZodError(error: z.ZodError): string { + return error.issues.map((issue) => issue.message).join("; "); } diff --git a/src/testing/globalConfig.tsx b/src/testing/globalConfig.tsx index 2c2b907a0..5d6f372a0 100644 --- a/src/testing/globalConfig.tsx +++ b/src/testing/globalConfig.tsx @@ -1,10 +1,10 @@ -import type { GlobalConfigAccessor, GlobalConfigShape, AsyncDataSource } from "../globalConfig"; +import type { GlobalConfig, GlobalConfigAccessor, JsonDataSource } from "../globalConfig"; import { createGlobalConfigAccessor, globalConfigSchema } from "../globalConfig"; import type { Logger } from "../logging"; import { createSilentLogger } from "./logging"; interface TestGlobalConfigAccessorOptions { - initialConfigData?: GlobalConfigShape; + initialConfigData?: GlobalConfig; logger?: Logger; } @@ -16,7 +16,7 @@ export function testGlobalConfigAccessor( options?: TestGlobalConfigAccessorOptions, ): GlobalConfigAccessor { let data = structuredClone(options?.initialConfigData ?? { installationId: crypto.randomUUID() }); - const source: AsyncDataSource = { + const source: JsonDataSource = { read: async () => data, write: async (d) => { data = globalConfigSchema.parse(d); From fb55ecbc65291ef2a9db95cd11695c549faec758 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 21 Jul 2026 14:08:01 -0400 Subject: [PATCH 04/13] refactor(globalConfig): swap from direct implementations to classes --- src/globalConfig/accessor.tsx | 58 +++++++++-------- src/globalConfig/index.tsx | 4 +- src/globalConfig/source.tsx | 96 +++++++++++++++-------------- src/handlers/config/config.test.tsx | 8 +-- src/index.ts | 8 +-- src/testing/globalConfig.tsx | 7 ++- 6 files changed, 97 insertions(+), 84 deletions(-) diff --git a/src/globalConfig/accessor.tsx b/src/globalConfig/accessor.tsx index 0ad0b3a9d..117b79de7 100644 --- a/src/globalConfig/accessor.tsx +++ b/src/globalConfig/accessor.tsx @@ -1,39 +1,43 @@ import { type GlobalConfig, type GlobalConfigAccessor, type JsonDataSource } from "./types"; import type { Logger } from "../logging"; -export interface GlobalConfigAccessorConfig { +type DefaultGlobalConfigAccessorConfig = { logger: Logger; source: JsonDataSource; -} +}; /** - * Creates a {@link GlobalConfigAccessor} backed by the given data source. + * Implements {@link GlobalConfigAccessor} backed by the given data source. * * @param config - The logger and DataSource to be used by the config. . * @returns A {@link GlobalConfigAccessor} instance. */ -export function createGlobalConfigAccessor( - config: GlobalConfigAccessorConfig, -): GlobalConfigAccessor { - config.logger.info(`creating global config accessor`); - let cachedConfig: GlobalConfig | undefined; - const accessor: GlobalConfigAccessor = { - get: async () => { - const logger = config.logger.child({ method: "get" }); - logger.debug("reading global config"); - if (cachedConfig) return cachedConfig; - logger.debug("failed to find cache, reading from source"); - cachedConfig = await config.source.read(); - return cachedConfig; - }, - - set: async (newConfig) => { - config.logger.child({ newConfig, method: "set" }).debug("writing global config"); - - cachedConfig = await config.source.write(newConfig); - return cachedConfig; - }, - }; - - return accessor; +export class DefaultGlobalConfigAccessor implements GlobalConfigAccessor { + private cachedConfig: GlobalConfig | undefined; + private readonly source: JsonDataSource; + private readonly logger: Logger; + + constructor(config: DefaultGlobalConfigAccessorConfig) { + this.logger = config.logger; + this.logger.info(`creating global config accessor`); + this.source = config.source; + } + + public async get(): Promise { + const logger = this.logger.child({ method: "get" }); + logger.debug("reading global config"); + + if (this.cachedConfig) return this.cachedConfig; + + logger.debug("failed to find cache, reading from source"); + this.cachedConfig = await this.source.read(); + return this.cachedConfig; + } + + public async set(newConfig: Record): Promise { + this.logger.child({ newConfig, method: "set" }).debug("writing global config"); + + this.cachedConfig = await this.source.write(newConfig); + return this.cachedConfig; + } } diff --git a/src/globalConfig/index.tsx b/src/globalConfig/index.tsx index a66aeaac0..952be3f27 100644 --- a/src/globalConfig/index.tsx +++ b/src/globalConfig/index.tsx @@ -1,4 +1,4 @@ export { type GlobalConfigAccessor, type GlobalConfig, type JsonDataSource } from "./types"; -export { createGlobalConfigAccessor } from "./accessor"; -export { createJsonFileDataSource } from "./source"; export { globalConfigSchema, isValidKey } from "./schemas"; +export { DefaultGlobalConfigAccessor } from "./accessor"; +export { JsonFileDataSource } from "./source"; diff --git a/src/globalConfig/source.tsx b/src/globalConfig/source.tsx index 27a29f871..a4db77ea4 100644 --- a/src/globalConfig/source.tsx +++ b/src/globalConfig/source.tsx @@ -4,7 +4,7 @@ import { dirname } from "path"; import type { Logger } from "../logging"; import type { JsonDataSource } from "./types"; -export interface JsonFileDataSourceConfig { +type JsonFileDataSourceConfig = { filePath: string; /** * Describes the data to write to the file if it does not exist or is deleted. @@ -12,7 +12,7 @@ export interface JsonFileDataSourceConfig { getDefaultData(): z.infer; schema: TSchema; logger: Logger; -} +}; function isNodeError(e: unknown): e is NodeJS.ErrnoException { return e instanceof Error && "code" in e; @@ -36,59 +36,65 @@ async function atomicWrite(filePath: string, data: string) { } /** - * Creates an {@link AsyncDataSource} backed by a JSON file on disk. + * Implements a {@link JsonDataSource} backed by a JSON file on disk. * * If the file does not exist, one will be created on first read with initialData or on write with the given data. * * @param config - File path, initial data, Zod schema, and logger. */ -export function createJsonFileDataSource( - config: JsonFileDataSourceConfig, -): JsonDataSource> { - const logger = config.logger.child({ filePath: config.filePath }); - logger.info(`creating data source from json file '${config.filePath}'`); +export class JsonFileDataSource implements JsonDataSource< + z.infer +> { + private readonly logger: Logger; + private readonly filePath: string; + private readonly schema: TSchema; + private readonly getDefaultData: () => z.infer; - const source: JsonDataSource> = { - read: async () => { - try { - const raw = await readFile(config.filePath, "utf-8"); - return config.schema.parse(JSON.parse(raw)); - } catch (e) { - // If the file doesn't exist, create one with the defaults. - if (isNodeError(e) && e.code === "ENOENT") { - logger.warn(`unable to find json file, creating default`); - return await source.write(config.getDefaultData()); - } - const error = e instanceof Error ? e : new Error(String(e)); + constructor(config: JsonFileDataSourceConfig) { + this.logger = config.logger; + this.filePath = config.filePath; + this.schema = config.schema; + this.getDefaultData = config.getDefaultData; + this.logger.info(`creating data source from json file '${config.filePath}'`); + } - logger - .child({ errorName: error.name, errorMessage: error.message }) - .error("failed to read config file"); - // TODO: swap to typed error. - throw new Error(`unable to read json file at '${config.filePath}'`); + public async read(): Promise> { + try { + const raw = await readFile(this.filePath, "utf8"); + return this.schema.parse(JSON.parse(raw)); + } catch (e) { + // If the file doesn't exist, create one with the defaults. + if (isNodeError(e) && e.code === "ENOENT") { + this.logger.warn(`unable to find json file, creating default`); + return await this.write(this.getDefaultData()); } - }, + const error = e instanceof Error ? e : new Error(String(e)); - write: async (data) => { - const parseDataResult = config.schema.safeParse(data); + this.logger + .child({ errorName: error.name, errorMessage: error.message }) + .error("failed to read config file"); + // TODO: swap to typed error. + throw new Error(`unable to read json file at '${this.filePath}'`); + } + } - if (!parseDataResult.success) { - logger - .child({ - errorName: parseDataResult.error.name, - errorMessage: parseDataResult.error.message, - }) - .error(`failed to validate data on write`); - // TODO: swap to validation. - throw new TypeError(z.prettifyError(parseDataResult.error)); - } + public async write(newData: unknown): Promise> { + const parseDataResult = this.schema.safeParse(newData); - const parsedData = parseDataResult.data; - await mkdir(dirname(config.filePath), { recursive: true }); - await atomicWrite(config.filePath, JSON.stringify(parsedData, null, 2)); - return parsedData; - }, - }; + if (!parseDataResult.success) { + this.logger + .child({ + errorName: parseDataResult.error.name, + errorMessage: parseDataResult.error.message, + }) + .error(`failed to validate data on write`); + // TODO: swap to typed error. + throw new TypeError(z.prettifyError(parseDataResult.error)); + } - return source; + const parsedData = parseDataResult.data; + await mkdir(dirname(this.filePath), { recursive: true }); + await atomicWrite(this.filePath, JSON.stringify(parsedData, null, 2)); + return parsedData; + } } diff --git a/src/handlers/config/config.test.tsx b/src/handlers/config/config.test.tsx index 2ad810866..c51df3665 100644 --- a/src/handlers/config/config.test.tsx +++ b/src/handlers/config/config.test.tsx @@ -5,9 +5,9 @@ import { tmpdir } from "node:os"; import { createRootHandler } from "../index"; import { createSilentLogger, TestCoreClient, testIO } from "../../testing"; import { - createGlobalConfigAccessor, + DefaultGlobalConfigAccessor, globalConfigSchema, - createJsonFileDataSource, + JsonFileDataSource, } from "../../globalConfig"; describe("config", () => { @@ -36,9 +36,9 @@ describe("config", () => { async function run(args: string[]): Promise { const io = testIO(); const logger = createSilentLogger(); - const globalConfigAccessor = createGlobalConfigAccessor({ + const globalConfigAccessor = new DefaultGlobalConfigAccessor({ logger, - source: createJsonFileDataSource({ + source: new JsonFileDataSource({ filePath: configPath, schema: globalConfigSchema, getDefaultData: () => initialConfig, diff --git a/src/index.ts b/src/index.ts index 49dda2d29..a8adfe5d4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,9 +13,9 @@ import { createRootHandler } from "./handlers"; import { createFileLogger, LOG_LEVEL } from "./logging"; import { runWithExitCode } from "./runnable"; import { - createGlobalConfigAccessor, + DefaultGlobalConfigAccessor, globalConfigSchema, - createJsonFileDataSource, + JsonFileDataSource, } from "./globalConfig"; process.exit( @@ -37,9 +37,9 @@ process.exit( }; try { - const globalConfigAccessor = createGlobalConfigAccessor({ + const globalConfigAccessor = new DefaultGlobalConfigAccessor({ logger: rootLogger.child({ module: "globalConfigAccessor" }), - source: createJsonFileDataSource({ + source: new JsonFileDataSource({ filePath: join(homedir(), ".agentcore", "config.json"), schema: globalConfigSchema, getDefaultData: () => ({ installationId: crypto.randomUUID() }), diff --git a/src/testing/globalConfig.tsx b/src/testing/globalConfig.tsx index 5d6f372a0..667f0d7cd 100644 --- a/src/testing/globalConfig.tsx +++ b/src/testing/globalConfig.tsx @@ -1,5 +1,5 @@ import type { GlobalConfig, GlobalConfigAccessor, JsonDataSource } from "../globalConfig"; -import { createGlobalConfigAccessor, globalConfigSchema } from "../globalConfig"; +import { DefaultGlobalConfigAccessor, globalConfigSchema } from "../globalConfig"; import type { Logger } from "../logging"; import { createSilentLogger } from "./logging"; @@ -23,5 +23,8 @@ export function testGlobalConfigAccessor( return data; }, }; - return createGlobalConfigAccessor({ source, logger: options?.logger ?? createSilentLogger() }); + return new DefaultGlobalConfigAccessor({ + source, + logger: options?.logger ?? createSilentLogger(), + }); } From 548979ddecbf05c6c6c86a63a72f0cf602c64982 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 21 Jul 2026 17:29:34 -0400 Subject: [PATCH 05/13] refactor: simplify global config implementation --- src/globalConfig/accessor.tsx | 38 +++++-- src/globalConfig/index.tsx | 4 +- src/globalConfig/source.tsx | 106 +++++++------------- src/globalConfig/types.tsx | 13 ++- src/handlers/config/config.test.tsx | 23 +---- src/handlers/harness/exec/exec.test.tsx | 10 +- src/handlers/harness/harness.test.tsx | 4 +- src/handlers/harness/invoke/invoke.test.tsx | 4 +- src/handlers/help.screen.test.tsx | 4 +- src/handlers/project/project.test.ts | 4 +- src/handlers/root.test.tsx | 5 +- src/handlers/runtime/runtime.test.tsx | 3 + src/index.ts | 12 +-- src/middleware/withRegion.test.tsx | 4 +- src/testing/globalConfig.tsx | 48 ++++----- src/testing/index.tsx | 2 +- src/testing/renderScreen.tsx | 4 +- src/tui/tui.test.tsx | 5 +- 18 files changed, 127 insertions(+), 166 deletions(-) diff --git a/src/globalConfig/accessor.tsx b/src/globalConfig/accessor.tsx index 117b79de7..1df441cac 100644 --- a/src/globalConfig/accessor.tsx +++ b/src/globalConfig/accessor.tsx @@ -1,26 +1,31 @@ -import { type GlobalConfig, type GlobalConfigAccessor, type JsonDataSource } from "./types"; +import { type GlobalConfig, type GlobalConfigAccessor, type ReadWriteJson } from "./types"; import type { Logger } from "../logging"; +import { globalConfigSchema } from "./schemas"; +import z from "zod"; type DefaultGlobalConfigAccessorConfig = { logger: Logger; - source: JsonDataSource; + json: ReadWriteJson; + filePath: string; }; /** - * Implements {@link GlobalConfigAccessor} backed by the given data source. + * Implements {@link GlobalConfigAccessor} backed by the given json source. * - * @param config - The logger and DataSource to be used by the config. . + * @param config - The logger and json datasource to be used by the config. . * @returns A {@link GlobalConfigAccessor} instance. */ export class DefaultGlobalConfigAccessor implements GlobalConfigAccessor { private cachedConfig: GlobalConfig | undefined; - private readonly source: JsonDataSource; + private readonly json: ReadWriteJson; + private readonly filePath: string; private readonly logger: Logger; constructor(config: DefaultGlobalConfigAccessorConfig) { this.logger = config.logger; + this.json = config.json; + this.filePath = config.filePath; this.logger.info(`creating global config accessor`); - this.source = config.source; } public async get(): Promise { @@ -28,16 +33,29 @@ export class DefaultGlobalConfigAccessor implements GlobalConfigAccessor { logger.debug("reading global config"); if (this.cachedConfig) return this.cachedConfig; - logger.debug("failed to find cache, reading from source"); - this.cachedConfig = await this.source.read(); - return this.cachedConfig; + + try { + this.cachedConfig = await this.json.read(this.filePath, globalConfigSchema); + return this.cachedConfig; + } catch (e) { + if (isFileNotFoundError(e)) return {}; + throw e; + } } public async set(newConfig: Record): Promise { this.logger.child({ newConfig, method: "set" }).debug("writing global config"); - this.cachedConfig = await this.source.write(newConfig); + const validatedConfig = globalConfigSchema.safeParse(newConfig); + if (!validatedConfig.success) { + throw new TypeError(z.prettifyError(validatedConfig.error)); + } + this.cachedConfig = await this.json.write(this.filePath, validatedConfig.data); return this.cachedConfig; } } + +function isFileNotFoundError(e: unknown): boolean { + return e instanceof Error && "code" in e && e.code === "ENOENT"; +} diff --git a/src/globalConfig/index.tsx b/src/globalConfig/index.tsx index 952be3f27..7af6c4b3c 100644 --- a/src/globalConfig/index.tsx +++ b/src/globalConfig/index.tsx @@ -1,4 +1,4 @@ -export { type GlobalConfigAccessor, type GlobalConfig, type JsonDataSource } from "./types"; +export { type GlobalConfigAccessor, type GlobalConfig, type ReadWriteJson } from "./types"; export { globalConfigSchema, isValidKey } from "./schemas"; export { DefaultGlobalConfigAccessor } from "./accessor"; -export { JsonFileDataSource } from "./source"; +export { FsReadWriteJson } from "./source"; diff --git a/src/globalConfig/source.tsx b/src/globalConfig/source.tsx index a4db77ea4..e02eb4b1e 100644 --- a/src/globalConfig/source.tsx +++ b/src/globalConfig/source.tsx @@ -1,100 +1,70 @@ import z from "zod"; -import { readFile, writeFile, mkdir, rename, rm } from "fs/promises"; -import { dirname } from "path"; +import { readFile, writeFile } from "fs/promises"; import type { Logger } from "../logging"; -import type { JsonDataSource } from "./types"; +import type { ReadWriteJson } from "./types"; -type JsonFileDataSourceConfig = { - filePath: string; - /** - * Describes the data to write to the file if it does not exist or is deleted. - */ - getDefaultData(): z.infer; - schema: TSchema; +type ReadWriteJsonConfig = { logger: Logger; }; -function isNodeError(e: unknown): e is NodeJS.ErrnoException { - return e instanceof Error && "code" in e; -} - -// node fs.write is not atomic, and its possible to leave behind corrupted files. -async function atomicWrite(filePath: string, data: string) { - const tempPath = `${filePath}.${crypto.randomUUID()}.tmp`; - - try { - await writeFile(tempPath, data, "utf8"); - await rename(tempPath, filePath); - } catch (e) { - try { - await rm(tempPath); - } catch { - // best effort - } - throw e; +// TODO: attach telemetry metadata to this error class. +class DeserializationError extends Error { + constructor(path: string, options?: { cause?: unknown }) { + super(`Failed to deserialize JSON at "${path}"`, options); + this.name = "DeserializationError"; } } /** - * Implements a {@link JsonDataSource} backed by a JSON file on disk. + * Implements {@link ReadWriteJson} through node fs. * - * If the file does not exist, one will be created on first read with initialData or on write with the given data. - * - * @param config - File path, initial data, Zod schema, and logger. + * @param config - logger */ -export class JsonFileDataSource implements JsonDataSource< - z.infer -> { +export class FsReadWriteJson implements ReadWriteJson { private readonly logger: Logger; - private readonly filePath: string; - private readonly schema: TSchema; - private readonly getDefaultData: () => z.infer; - constructor(config: JsonFileDataSourceConfig) { + constructor(config: ReadWriteJsonConfig) { this.logger = config.logger; - this.filePath = config.filePath; - this.schema = config.schema; - this.getDefaultData = config.getDefaultData; - this.logger.info(`creating data source from json file '${config.filePath}'`); } - public async read(): Promise> { + private async readJson(filePath: string): Promise { + const raw = await readFile(filePath, "utf8"); + try { - const raw = await readFile(this.filePath, "utf8"); - return this.schema.parse(JSON.parse(raw)); + return JSON.parse(raw); } catch (e) { - // If the file doesn't exist, create one with the defaults. - if (isNodeError(e) && e.code === "ENOENT") { - this.logger.warn(`unable to find json file, creating default`); - return await this.write(this.getDefaultData()); - } const error = e instanceof Error ? e : new Error(String(e)); - this.logger - .child({ errorName: error.name, errorMessage: error.message }) - .error("failed to read config file"); - // TODO: swap to typed error. - throw new Error(`unable to read json file at '${this.filePath}'`); + .child({ filePath, errorName: error.name, errorMessage: error.message }) + .error(`failed to parse json file`); + throw new DeserializationError(filePath, { cause: e }); } } - public async write(newData: unknown): Promise> { - const parseDataResult = this.schema.safeParse(newData); + public async read( + filePath: string, + schema: TSchema, + ): Promise> { + const data = await this.readJson(filePath); + const parseResult = schema.safeParse(data); - if (!parseDataResult.success) { + if (!parseResult.success) { this.logger .child({ - errorName: parseDataResult.error.name, - errorMessage: parseDataResult.error.message, + filePath, + errorName: parseResult.error.name, + errorMessage: parseResult.error.message, }) - .error(`failed to validate data on write`); - // TODO: swap to typed error. - throw new TypeError(z.prettifyError(parseDataResult.error)); + .error(`failed to validate parsed json file`); + throw new DeserializationError(filePath, { cause: parseResult.error }); } - const parsedData = parseDataResult.data; - await mkdir(dirname(this.filePath), { recursive: true }); - await atomicWrite(this.filePath, JSON.stringify(parsedData, null, 2)); - return parsedData; + return parseResult.data; + } + + public async write(filePath: string, data: TData): Promise { + const contents = JSON.stringify(data, undefined, 2); + await writeFile(filePath, contents); + return data; } } diff --git a/src/globalConfig/types.tsx b/src/globalConfig/types.tsx index 03efba31a..d85e35d92 100644 --- a/src/globalConfig/types.tsx +++ b/src/globalConfig/types.tsx @@ -9,13 +9,12 @@ export interface GlobalConfigAccessor { /** Returns the current global config. */ get(): Promise; /** Validates and persists a new config. Throws on invalid shape. */ - set(newConfig: unknown): Promise; + set(newConfig: Record): Promise; } -/** Generic read/write source for a typed JSON object. */ -export interface JsonDataSource { - /** Reads data from the source. */ - read(): Promise; - /** Validates and writes data to the source. Throws on invalid shape. */ - write(data: unknown): Promise; +export interface ReadWriteJson { + /** Reads data from the given file */ + read(filePath: string, schema: TSchema): Promise>; + /** Writes data to the given file */ + write(filePath: string, data: TData): Promise; } diff --git a/src/handlers/config/config.test.tsx b/src/handlers/config/config.test.tsx index c51df3665..9aa738f61 100644 --- a/src/handlers/config/config.test.tsx +++ b/src/handlers/config/config.test.tsx @@ -4,11 +4,7 @@ import { mkdtemp, rm, writeFile, readFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { createRootHandler } from "../index"; import { createSilentLogger, TestCoreClient, testIO } from "../../testing"; -import { - DefaultGlobalConfigAccessor, - globalConfigSchema, - JsonFileDataSource, -} from "../../globalConfig"; +import { DefaultGlobalConfigAccessor, FsReadWriteJson } from "../../globalConfig"; describe("config", () => { let tempDir: string; @@ -19,10 +15,6 @@ describe("config", () => { installationId: "550e8400-e29b-41d4-a716-446655440000", }; - const initialConfig = { - installationId: "650e8400-e29b-41d4-a716-446655440000", - }; - beforeEach(async () => { tempDir = await mkdtemp(join(tmpdir(), "agentcore-config-test-")); configPath = join(tempDir, "config.json"); @@ -38,10 +30,8 @@ describe("config", () => { const logger = createSilentLogger(); const globalConfigAccessor = new DefaultGlobalConfigAccessor({ logger, - source: new JsonFileDataSource({ - filePath: configPath, - schema: globalConfigSchema, - getDefaultData: () => initialConfig, + filePath: configPath, + json: new FsReadWriteJson({ logger, }), }); @@ -140,7 +130,7 @@ describe("config", () => { telemetry: { enabled: "not-a-bool", endpoint: "https://good.com" }, }), ); - await expect(run(["telemetry.endpoint"])).rejects.toThrow("unable to read"); + await expect(run(["telemetry.endpoint"])).rejects.toThrow("Failed to deserialize"); }); test("ignores unsupported fields in the config file", async () => { @@ -159,7 +149,7 @@ describe("config", () => { test("throws clear error on invalid json", async () => { await writeFile(configPath, "not { valid json"); // TODO: assert on error type - await expect(run([])).rejects.toThrow("unable to read"); + await expect(run([])).rejects.toThrow("Failed to deserialize"); }); test("creates default config if it does not exist", async () => { @@ -171,8 +161,5 @@ describe("config", () => { const readOutput = await run(["telemetry.endpoint"]); expect(JSON.parse(readOutput)).toBe(newEndpoint); - - const installationIdReadOutput = await run(["installationId"]); - expect(JSON.parse(installationIdReadOutput)).toBe(initialConfig.installationId); }); }); diff --git a/src/handlers/harness/exec/exec.test.tsx b/src/handlers/harness/exec/exec.test.tsx index 10ca89e5f..96a56d4f0 100644 --- a/src/handlers/harness/exec/exec.test.tsx +++ b/src/handlers/harness/exec/exec.test.tsx @@ -5,12 +5,8 @@ import type { } from "@aws-sdk/client-bedrock-agentcore"; import type { GetHarnessResponse } from "@aws-sdk/client-bedrock-agentcore-control"; import { createRootHandler } from "../../index"; -import { - createSilentLogger, - TestCoreClient, - testIO, - testGlobalConfigAccessor, -} from "../../../testing"; +import { createSilentLogger, TestCoreClient, testIO } from "../../../testing"; +import { TestGlobalConfigAccessor } from "../../../testing/"; // Command-flow tests for `harness exec`, driven through the real root handler. // Like the invoke suite, these use a TestCoreClient because the command @@ -39,7 +35,7 @@ async function run(args: string[], configure?: (core: TestCoreClient) => void) { const root = createRootHandler(core, { io: io.io, logger: createSilentLogger(), - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }); await root.route(["node", "agentcore", ...args, "--region", "us-west-2"]); return { core, stdout: io.stdout() }; diff --git a/src/handlers/harness/harness.test.tsx b/src/handlers/harness/harness.test.tsx index c5e9cda27..68ef8e5f7 100644 --- a/src/handlers/harness/harness.test.tsx +++ b/src/handlers/harness/harness.test.tsx @@ -7,8 +7,8 @@ import { fixtureFactories, isRecording, matchGolden, + TestGlobalConfigAccessor, testIO, - testGlobalConfigAccessor, } from "../../testing"; // End-to-end command-flow tests for the `harness` subtree. @@ -36,7 +36,7 @@ async function run(args: string[]): Promise { const root = createRootHandler(core, { io: io.io, logger: createSilentLogger(), - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }); await root.route(["node", "agentcore", ...args, "--region", REGION]); return io.stdout(); diff --git a/src/handlers/harness/invoke/invoke.test.tsx b/src/handlers/harness/invoke/invoke.test.tsx index 784404706..2ff072b4d 100644 --- a/src/handlers/harness/invoke/invoke.test.tsx +++ b/src/handlers/harness/invoke/invoke.test.tsx @@ -8,8 +8,8 @@ import { createRootHandler } from "../../index"; import { createSilentLogger, TestCoreClient, + TestGlobalConfigAccessor, testIO, - testGlobalConfigAccessor, } from "../../../testing"; // Command-flow tests for `harness invoke`, driven through the real root handler @@ -49,7 +49,7 @@ async function run(args: string[], configure?: (core: TestCoreClient) => void) { const root = createRootHandler(core, { io: io.io, logger: createSilentLogger(), - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }); await root.route(["node", "agentcore", ...args, "--region", "us-west-2"]); return { core, stdout: io.stdout() }; diff --git a/src/handlers/help.screen.test.tsx b/src/handlers/help.screen.test.tsx index bab0064e8..f2792e21b 100644 --- a/src/handlers/help.screen.test.tsx +++ b/src/handlers/help.screen.test.tsx @@ -4,7 +4,7 @@ import { render, cleanup } from "ink-testing-library"; import { ValueContext, compile, CommandKey } from "../router"; import { createRootHandler } from "./index"; import { HelpScreen } from "./screen"; -import { createSilentLogger, TestCoreClient, testIO, testGlobalConfigAccessor } from "../testing"; +import { createSilentLogger, TestCoreClient, TestGlobalConfigAccessor, testIO } from "../testing"; afterEach(cleanup); @@ -19,7 +19,7 @@ describe("HelpScreen", () => { createRootHandler(new TestCoreClient(), { io: testIO().io, logger: createSilentLogger(), - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }), ValueContext.EmptyContext(), ); diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index 10067936c..8d9540d0d 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -3,7 +3,7 @@ import { createRootHandler } from "../index"; import { createSilentLogger, TestCoreClient, - testGlobalConfigAccessor, + TestGlobalConfigAccessor, testIO, } from "../../testing"; @@ -11,7 +11,7 @@ async function run(args: string[]): Promise { const io = testIO(); const root = createRootHandler(new TestCoreClient(), { io: io.io, - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), logger: createSilentLogger(), }); await root.route(["node", "agentcore", "project", ...args]); diff --git a/src/handlers/root.test.tsx b/src/handlers/root.test.tsx index 1c7817420..b0e10d403 100644 --- a/src/handlers/root.test.tsx +++ b/src/handlers/root.test.tsx @@ -1,14 +1,13 @@ import { test, expect, describe } from "bun:test"; import { createRootHandler } from "./index"; -import { createSilentLogger, TestCoreClient, testIO } from "../testing"; -import { testGlobalConfigAccessor } from "../testing"; +import { createSilentLogger, TestCoreClient, TestGlobalConfigAccessor, testIO } from "../testing"; describe("createRootHandler", () => { test("builds the agentcore command tree with its subcommands", () => { const root = createRootHandler(new TestCoreClient(), { io: testIO().io, logger: createSilentLogger(), - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }); expect(root.name()).toBe("agentcore"); expect(root.children().map((c) => c.name())).toEqual([ diff --git a/src/handlers/runtime/runtime.test.tsx b/src/handlers/runtime/runtime.test.tsx index 5685d8619..79748685c 100644 --- a/src/handlers/runtime/runtime.test.tsx +++ b/src/handlers/runtime/runtime.test.tsx @@ -3,6 +3,7 @@ import { join } from "node:path"; import { CoreClient } from "../../core"; import { createSilentLogger, fixtureFactories, matchGolden, testIO } from "../../testing"; import { createRootHandler } from "../index"; +import { TestGlobalConfigAccessor } from "../../testing/globalConfig"; const REGION = "us-west-2"; const FIXTURES = join(import.meta.dir, "__fixtures__"); @@ -24,6 +25,7 @@ async function run(args: string[]): Promise { const root = createRootHandler(createFixtureCore(), { io: io.io, logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }); await root.route(["node", "agentcore", ...args, "--region", REGION]); @@ -35,6 +37,7 @@ describe("runtime command hierarchy", () => { const root = createRootHandler(createFixtureCore(), { io: testIO().io, logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }); const runtime = root.children().find((child) => child.name() === "runtime"); diff --git a/src/index.ts b/src/index.ts index a8adfe5d4..178d6997d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,11 +12,7 @@ import { createControlClient, createDataClient, createIamClient } from "./core/f import { createRootHandler } from "./handlers"; import { createFileLogger, LOG_LEVEL } from "./logging"; import { runWithExitCode } from "./runnable"; -import { - DefaultGlobalConfigAccessor, - globalConfigSchema, - JsonFileDataSource, -} from "./globalConfig"; +import { DefaultGlobalConfigAccessor, FsReadWriteJson } from "./globalConfig"; process.exit( await runWithExitCode(async (argv: string[]) => { @@ -39,10 +35,8 @@ process.exit( try { const globalConfigAccessor = new DefaultGlobalConfigAccessor({ logger: rootLogger.child({ module: "globalConfigAccessor" }), - source: new JsonFileDataSource({ - filePath: join(homedir(), ".agentcore", "config.json"), - schema: globalConfigSchema, - getDefaultData: () => ({ installationId: crypto.randomUUID() }), + filePath: join(homedir(), ".agentcore", "config.json"), + json: new FsReadWriteJson({ logger: rootLogger.child({ module: "jsonDataSource" }), }), }); diff --git a/src/middleware/withRegion.test.tsx b/src/middleware/withRegion.test.tsx index 46c97c6c9..1404f93ac 100644 --- a/src/middleware/withRegion.test.tsx +++ b/src/middleware/withRegion.test.tsx @@ -3,7 +3,7 @@ import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createRootHandler } from "../handlers"; -import { TestCoreClient, testGlobalConfigAccessor, testIO } from "../testing"; +import { TestCoreClient, TestGlobalConfigAccessor, testIO } from "../testing"; import { createSilentLogger } from "../testing/"; // writeConfigFile writes an AWS shared-config file with the given contents to a @@ -27,7 +27,7 @@ async function resolvedRegion(args: string[]): Promise { const root = createRootHandler(core, { io: testIO().io, logger: createSilentLogger(), - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }); await root.route(["node", "agentcore", "harness", "list", "--json", ...args]); const call = core.harness.calls.at(-1); diff --git a/src/testing/globalConfig.tsx b/src/testing/globalConfig.tsx index 667f0d7cd..5623b1ca1 100644 --- a/src/testing/globalConfig.tsx +++ b/src/testing/globalConfig.tsx @@ -1,30 +1,26 @@ -import type { GlobalConfig, GlobalConfigAccessor, JsonDataSource } from "../globalConfig"; -import { DefaultGlobalConfigAccessor, globalConfigSchema } from "../globalConfig"; -import type { Logger } from "../logging"; -import { createSilentLogger } from "./logging"; - -interface TestGlobalConfigAccessorOptions { - initialConfigData?: GlobalConfig; - logger?: Logger; -} +import type { GlobalConfig, GlobalConfigAccessor } from "../globalConfig"; +import { globalConfigSchema } from "../globalConfig"; /** - * In-memory GlobalConfigAccessor for tests. Injects a plain-object - * AsyncDataSource into the real createGlobalConfigAccessor + * In-memory GlobalConfigAccessor for tests. */ -export function testGlobalConfigAccessor( - options?: TestGlobalConfigAccessorOptions, -): GlobalConfigAccessor { - let data = structuredClone(options?.initialConfigData ?? { installationId: crypto.randomUUID() }); - const source: JsonDataSource = { - read: async () => data, - write: async (d) => { - data = globalConfigSchema.parse(d); - return data; - }, - }; - return new DefaultGlobalConfigAccessor({ - source, - logger: options?.logger ?? createSilentLogger(), - }); +type TestGlobalConfigAccessorOptions = { + initialConfigData?: GlobalConfig; +}; + +export class TestGlobalConfigAccessor implements GlobalConfigAccessor { + private configData: GlobalConfig; + + constructor(options?: TestGlobalConfigAccessorOptions) { + this.configData = options?.initialConfigData ?? {}; + } + + public async get(): Promise { + return this.configData; + } + + public async set(newConfig: Record): Promise { + this.configData = globalConfigSchema.parse(newConfig); + return this.configData; + } } diff --git a/src/testing/index.tsx b/src/testing/index.tsx index 9a60d1817..c45cdc611 100644 --- a/src/testing/index.tsx +++ b/src/testing/index.tsx @@ -13,4 +13,4 @@ export { type RenderScreenResult, } from "./renderScreen"; export { createSilentLogger, assertLogsMatch, type LogQuery } from "./logging"; -export { testGlobalConfigAccessor } from "./globalConfig"; +export { TestGlobalConfigAccessor } from "./globalConfig"; diff --git a/src/testing/renderScreen.tsx b/src/testing/renderScreen.tsx index daf581ceb..b7ac3939d 100644 --- a/src/testing/renderScreen.tsx +++ b/src/testing/renderScreen.tsx @@ -9,7 +9,7 @@ import { TestCoreClient } from "./TestCoreClient"; import { testIO } from "./testIO"; import { tick, waitFor } from "./timing"; import { createSilentLogger } from "./logging"; -import { testGlobalConfigAccessor } from "./globalConfig"; +import { TestGlobalConfigAccessor } from "./globalConfig"; // TUI test harness. // @@ -32,7 +32,7 @@ function baseContext(core: TestCoreClient): Context { createRootHandler(core, { io: testIO().io, logger: createSilentLogger(), - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }), ValueContext.EmptyContext(), ); diff --git a/src/tui/tui.test.tsx b/src/tui/tui.test.tsx index 93e6a239a..ca34a7e6b 100644 --- a/src/tui/tui.test.tsx +++ b/src/tui/tui.test.tsx @@ -1,8 +1,7 @@ import { test, expect, describe } from "bun:test"; import { createRootHandler } from "../handlers"; import { renderJson } from "./index"; -import { createSilentLogger, TestCoreClient, testIO } from "../testing"; -import { testGlobalConfigAccessor } from "../testing"; +import { createSilentLogger, TestCoreClient, TestGlobalConfigAccessor, testIO } from "../testing"; describe("renderJson", () => { test("pretty-prints a value as indented JSON to the given writer", () => { @@ -22,7 +21,7 @@ describe("--json short-circuits the TUI", () => { const root = createRootHandler(new TestCoreClient(), { io: io.io, logger: createSilentLogger(), - globalConfigAccessor: testGlobalConfigAccessor(), + globalConfigAccessor: new TestGlobalConfigAccessor(), }); await root.route(["node", "agentcore", ...args, "--json"]); return io.stdout(); From fc33e99f6e547d984662e1506c9c77ec5c7ef3a6 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 22 Jul 2026 13:32:08 +0000 Subject: [PATCH 06/13] refactor(globalConfig): rename json file --- src/globalConfig/index.tsx | 2 +- src/globalConfig/{source.tsx => json.tsx} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/globalConfig/{source.tsx => json.tsx} (100%) diff --git a/src/globalConfig/index.tsx b/src/globalConfig/index.tsx index 7af6c4b3c..70ce34744 100644 --- a/src/globalConfig/index.tsx +++ b/src/globalConfig/index.tsx @@ -1,4 +1,4 @@ export { type GlobalConfigAccessor, type GlobalConfig, type ReadWriteJson } from "./types"; export { globalConfigSchema, isValidKey } from "./schemas"; export { DefaultGlobalConfigAccessor } from "./accessor"; -export { FsReadWriteJson } from "./source"; +export { FsReadWriteJson } from "./json"; diff --git a/src/globalConfig/source.tsx b/src/globalConfig/json.tsx similarity index 100% rename from src/globalConfig/source.tsx rename to src/globalConfig/json.tsx From d0054a421c17b1bc7cb48a017c26648c3cab62de Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 22 Jul 2026 13:43:35 +0000 Subject: [PATCH 07/13] refactor(config): simplify handler implementation --- src/handlers/config/handler.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/handlers/config/handler.tsx b/src/handlers/config/handler.tsx index 8785a3842..344df9c3b 100644 --- a/src/handlers/config/handler.tsx +++ b/src/handlers/config/handler.tsx @@ -29,24 +29,25 @@ export const createConfigHandler = () => const globalConfig = await globalConfigAccessor.get(); // print entire config when key is missing. if (!args.key) { - if (globalConfig !== undefined) jsonRenderer.renderJson(globalConfig); + jsonRenderer.renderJson(globalConfig); return; } + if (!isValidKey(args.key)) throw new TypeError(`invalid key ${args.key} for global config`); + // print entire value at key when value is missing. if (!args.value) { const scopedConfig = getAtPath(globalConfig, args.key); - if (scopedConfig === undefined && !isValidKey(args.key)) - throw new TypeError(`invalid key ${args.key} for global config`); if (scopedConfig !== undefined) jsonRenderer.renderJson(scopedConfig); return; } // update value at key with given value. - const rawUpdatedConfig = setAtPath(globalConfig, args.key, args.value); - const updatedConfig = await globalConfigAccessor.set(rawUpdatedConfig); + const updatedConfig = await globalConfigAccessor.set( + setAtPath(globalConfig, args.key, args.value), + ); const updatedScopedConfig = getAtPath(updatedConfig, args.key); - jsonRenderer.renderJson(updatedScopedConfig); + if (updatedScopedConfig !== undefined) jsonRenderer.renderJson(updatedScopedConfig); }, }); From 73978ec58071df2c8a690726d080a731d2fc39b4 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 22 Jul 2026 13:47:59 +0000 Subject: [PATCH 08/13] refactor(globalConfig): adjust log message to avoid failure language in non-failure path --- src/globalConfig/accessor.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/globalConfig/accessor.tsx b/src/globalConfig/accessor.tsx index 1df441cac..fe7daf10d 100644 --- a/src/globalConfig/accessor.tsx +++ b/src/globalConfig/accessor.tsx @@ -33,7 +33,7 @@ export class DefaultGlobalConfigAccessor implements GlobalConfigAccessor { logger.debug("reading global config"); if (this.cachedConfig) return this.cachedConfig; - logger.debug("failed to find cache, reading from source"); + logger.debug("no config cached, reading from source"); try { this.cachedConfig = await this.json.read(this.filePath, globalConfigSchema); From 16c79227173c6e616a61dde23b53c198bd8d7050 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 22 Jul 2026 22:42:37 +0000 Subject: [PATCH 09/13] refactor(globalConfig): move coercion to the handler layer --- src/globalConfig/accessor.tsx | 2 +- src/globalConfig/index.tsx | 2 +- src/globalConfig/schemas.tsx | 91 +++++++++++++++------------------ src/globalConfig/types.tsx | 2 +- src/handlers/config/handler.tsx | 37 +++++++++++--- 5 files changed, 74 insertions(+), 60 deletions(-) diff --git a/src/globalConfig/accessor.tsx b/src/globalConfig/accessor.tsx index fe7daf10d..0867f2524 100644 --- a/src/globalConfig/accessor.tsx +++ b/src/globalConfig/accessor.tsx @@ -44,7 +44,7 @@ export class DefaultGlobalConfigAccessor implements GlobalConfigAccessor { } } - public async set(newConfig: Record): Promise { + public async set(newConfig: GlobalConfig): Promise { this.logger.child({ newConfig, method: "set" }).debug("writing global config"); const validatedConfig = globalConfigSchema.safeParse(newConfig); diff --git a/src/globalConfig/index.tsx b/src/globalConfig/index.tsx index 70ce34744..0f6a6a76a 100644 --- a/src/globalConfig/index.tsx +++ b/src/globalConfig/index.tsx @@ -1,4 +1,4 @@ export { type GlobalConfigAccessor, type GlobalConfig, type ReadWriteJson } from "./types"; -export { globalConfigSchema, isValidKey } from "./schemas"; +export { globalConfigSchema, getGlobalConfigKeys, getGlobalConfigValueType } from "./schemas"; export { DefaultGlobalConfigAccessor } from "./accessor"; export { FsReadWriteJson } from "./json"; diff --git a/src/globalConfig/schemas.tsx b/src/globalConfig/schemas.tsx index 3accbf148..9a6d583a4 100644 --- a/src/globalConfig/schemas.tsx +++ b/src/globalConfig/schemas.tsx @@ -1,53 +1,20 @@ import z from "zod"; -/** Zod boolean schema that coerces string values (`"true"`, `"1"`, `"false"`, `"0"`) to booleans. */ -const coercedBoolean = z.preprocess((val) => { - if (typeof val === "string") { - if (val === "true" || val === "1") return true; - if (val === "false" || val === "0") return false; - } - return val; -}, z.boolean()); - -/** Creates a Zod object schema that coerces JSON strings into objects before validation. */ -const coerceObject = (shape: TShape) => - z.preprocess((val) => { - if (typeof val === "string") { - try { - return JSON.parse(val); - } catch { - return val; - } - } - return val; - }, z.object(shape)); - /** * Schema for the global config file. {@link GlobalConfigAccessor} is built from this definition. * All fields should be optional; missing fields should be overridden with reasonable defaults in the consuming code. */ export const globalConfigSchema = z.object({ - telemetry: coerceObject({ - enabled: coercedBoolean.optional(), - endpoint: z.string().optional(), - audit: coercedBoolean.optional(), - }).optional(), + telemetry: z + .object({ + enabled: z.boolean().optional(), + endpoint: z.string().optional(), + audit: z.boolean().optional(), + }) + .optional(), installationId: z.uuid().optional(), }); -/** Executes `fn` on first call and caches the result for subsequent calls. */ -function once(fn: () => T): () => T { - let result: T; - let called = false; - return () => { - if (!called) { - result = fn(); - called = true; - } - return result; - }; -} - /** * Returns the internal def for a zod schema, which contains the `type` discriminant * and structural fields like `innerType`, `shape`, `out`, etc. @@ -84,17 +51,17 @@ function unwrapSchema(schema: z.ZodType): z.ZodType { } /** Recursively collects all valid dot-notation paths from a ZodObject schema. */ -function buildKeySet(schema: z.ZodObject, prefix = ""): Set { - const paths = new Set(); +function buildKeyList(schema: z.ZodObject, prefix = ""): string[] { + const paths: string[] = []; for (const key of schema.keyof().options) { const fullPath = prefix ? `${prefix}.${key}` : key; - paths.add(fullPath); + paths.push(fullPath); const inner = unwrapSchema(schema.shape[key]); if (getDef(inner).type === "object") { - for (const nested of buildKeySet(inner as z.ZodObject, fullPath)) { - paths.add(nested); + for (const nested of buildKeyList(inner as z.ZodObject, fullPath)) { + paths.push(nested); } } } @@ -102,9 +69,35 @@ function buildKeySet(schema: z.ZodObject, prefix = ""): Set { return paths; } -const getValidKeys = once(() => buildKeySet(globalConfigSchema)); +/** Returns all valid dot-notation config keys (e.g. `"telemetry.enabled"`). */ +export function getGlobalConfigKeys(): string[] { + return buildKeyList(globalConfigSchema); +} + +/** Walks a key path through nested object schemas, returning the schema at the leaf or `undefined` if the path is invalid. */ +function getSchemaForPath(keys: string[], schema: z.ZodType): z.ZodType | undefined { + if (keys.length === 0) { + return schema; + } + + const [key, ...rest] = keys; + + const unwrapped = unwrapSchema(schema); + const def = getDef(unwrapped); + if (def.type !== "object" || !def.shape) { + return undefined; + } + + const next = def.shape[key!]; + if (!next) { + return undefined; + } + + return getSchemaForPath(rest, next); +} -/** Returns whether a dot-notation path is a known key in {@link globalConfigSchema}. */ -export function isValidKey(path: string): boolean { - return getValidKeys().has(path); +/** Returns the base zod type name (e.g. `"boolean"`, `"string"`) for the schema at `path`, or `undefined` if the path is invalid. */ +export function getGlobalConfigValueType(path: string): z.ZodType["def"]["type"] | undefined { + const schema = getSchemaForPath(path.split("."), globalConfigSchema); + return schema ? unwrapSchema(schema).def.type : undefined; } diff --git a/src/globalConfig/types.tsx b/src/globalConfig/types.tsx index d85e35d92..e3d10b499 100644 --- a/src/globalConfig/types.tsx +++ b/src/globalConfig/types.tsx @@ -9,7 +9,7 @@ export interface GlobalConfigAccessor { /** Returns the current global config. */ get(): Promise; /** Validates and persists a new config. Throws on invalid shape. */ - set(newConfig: Record): Promise; + set(newConfig: GlobalConfig): Promise; } export interface ReadWriteJson { diff --git a/src/handlers/config/handler.tsx b/src/handlers/config/handler.tsx index 344df9c3b..b3ba971d7 100644 --- a/src/handlers/config/handler.tsx +++ b/src/handlers/config/handler.tsx @@ -1,7 +1,11 @@ import z from "zod"; import { createHandler, argument, GlobalConfigAccessorKey } from "../../router"; import { JsonRendererKey } from "../../tui"; -import { isValidKey } from "../../globalConfig"; +import { + getGlobalConfigKeys, + getGlobalConfigValueType, + type GlobalConfig, +} from "../../globalConfig"; /* * read/write global configuration values. ex. telemetry settings, endpoint overrides, etc. @@ -18,7 +22,7 @@ export const createConfigHandler = () => argument( "key", "config key in JSON path notation (e.g. telemetry.enabled)", - z.string().optional(), + z.enum(getGlobalConfigKeys()).optional(), ), argument("value", "value to set for the key", z.string().optional()), ], @@ -33,24 +37,41 @@ export const createConfigHandler = () => return; } - if (!isValidKey(args.key)) throw new TypeError(`invalid key ${args.key} for global config`); - // print entire value at key when value is missing. if (!args.value) { const scopedConfig = getAtPath(globalConfig, args.key); if (scopedConfig !== undefined) jsonRenderer.renderJson(scopedConfig); return; } + const coercedValue = coerceValue(args.key, args.value); + // update value at key with given value. - const updatedConfig = await globalConfigAccessor.set( - setAtPath(globalConfig, args.key, args.value), + await globalConfigAccessor.set( + setAtPath(globalConfig, args.key, coercedValue) as GlobalConfig, ); - const updatedScopedConfig = getAtPath(updatedConfig, args.key); - if (updatedScopedConfig !== undefined) jsonRenderer.renderJson(updatedScopedConfig); + jsonRenderer.renderJson(coercedValue); }, }); +function coerceValue(key: string, value: string): unknown { + const valueType = getGlobalConfigValueType(key); + if (!valueType) { + return value; + } + switch (valueType) { + case "boolean": + case "object": + try { + return JSON.parse(value); + } catch { + return value; + } + default: + return value; + } +} + /** Type guard that narrows `value` to a plain object record. */ function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); From b3411e779b2f4c58f1b2b340e8751d0e93e4e717 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 22 Jul 2026 23:06:23 +0000 Subject: [PATCH 10/13] feat: add docstring to missing helper functions --- src/handlers/config/handler.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/handlers/config/handler.tsx b/src/handlers/config/handler.tsx index b3ba971d7..a54181b10 100644 --- a/src/handlers/config/handler.tsx +++ b/src/handlers/config/handler.tsx @@ -54,6 +54,7 @@ export const createConfigHandler = () => }, }); +/** Coerces a raw CLI string to the type expected by the config schema at `key`. */ function coerceValue(key: string, value: string): unknown { const valueType = getGlobalConfigValueType(key); if (!valueType) { From 10dece83b7caee6dd78a967b4fd55778ff94f479 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 22 Jul 2026 23:15:00 +0000 Subject: [PATCH 11/13] refactor(globalConfig): use a shared map for both key and type lookups --- src/globalConfig/schemas.tsx | 49 +++++++++++------------------------- 1 file changed, 15 insertions(+), 34 deletions(-) diff --git a/src/globalConfig/schemas.tsx b/src/globalConfig/schemas.tsx index 9a6d583a4..03684d761 100644 --- a/src/globalConfig/schemas.tsx +++ b/src/globalConfig/schemas.tsx @@ -50,54 +50,35 @@ function unwrapSchema(schema: z.ZodType): z.ZodType { } } -/** Recursively collects all valid dot-notation paths from a ZodObject schema. */ -function buildKeyList(schema: z.ZodObject, prefix = ""): string[] { - const paths: string[] = []; +/** Recursively builds a map of dot-notation paths to their leaf schemas. */ +function buildSchemaMap(schema: z.ZodObject, prefix = ""): Map { + const map = new Map(); for (const key of schema.keyof().options) { const fullPath = prefix ? `${prefix}.${key}` : key; - paths.push(fullPath); + const value = schema.shape[key]; + map.set(fullPath, value); - const inner = unwrapSchema(schema.shape[key]); + const inner = unwrapSchema(value); if (getDef(inner).type === "object") { - for (const nested of buildKeyList(inner as z.ZodObject, fullPath)) { - paths.push(nested); + for (const [nested, s] of buildSchemaMap(inner as z.ZodObject, fullPath)) { + map.set(nested, s); } } } - return paths; + return map; } +const globalConfigSchemaMap = buildSchemaMap(globalConfigSchema); + /** Returns all valid dot-notation config keys (e.g. `"telemetry.enabled"`). */ export function getGlobalConfigKeys(): string[] { - return buildKeyList(globalConfigSchema); -} - -/** Walks a key path through nested object schemas, returning the schema at the leaf or `undefined` if the path is invalid. */ -function getSchemaForPath(keys: string[], schema: z.ZodType): z.ZodType | undefined { - if (keys.length === 0) { - return schema; - } - - const [key, ...rest] = keys; - - const unwrapped = unwrapSchema(schema); - const def = getDef(unwrapped); - if (def.type !== "object" || !def.shape) { - return undefined; - } - - const next = def.shape[key!]; - if (!next) { - return undefined; - } - - return getSchemaForPath(rest, next); + return [...globalConfigSchemaMap.keys()]; } /** Returns the base zod type name (e.g. `"boolean"`, `"string"`) for the schema at `path`, or `undefined` if the path is invalid. */ -export function getGlobalConfigValueType(path: string): z.ZodType["def"]["type"] | undefined { - const schema = getSchemaForPath(path.split("."), globalConfigSchema); - return schema ? unwrapSchema(schema).def.type : undefined; +export function getGlobalConfigValueType(path: string): string | undefined { + const schema = globalConfigSchemaMap.get(path); + return schema ? getDef(unwrapSchema(schema)).type : undefined; } From cd99d77a0b89095de4981600a0dd02f638fe0c55 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 22 Jul 2026 23:23:21 +0000 Subject: [PATCH 12/13] fix(globalConfig): add stronger typing on type fetch --- src/globalConfig/schemas.tsx | 4 ++-- src/handlers/config/handler.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/globalConfig/schemas.tsx b/src/globalConfig/schemas.tsx index 03684d761..89374ba0d 100644 --- a/src/globalConfig/schemas.tsx +++ b/src/globalConfig/schemas.tsx @@ -78,7 +78,7 @@ export function getGlobalConfigKeys(): string[] { } /** Returns the base zod type name (e.g. `"boolean"`, `"string"`) for the schema at `path`, or `undefined` if the path is invalid. */ -export function getGlobalConfigValueType(path: string): string | undefined { +export function getGlobalConfigValueType(path: string): z.ZodType["def"]["type"] | undefined { const schema = globalConfigSchemaMap.get(path); - return schema ? getDef(unwrapSchema(schema)).type : undefined; + return schema ? schema.def.type : undefined; } diff --git a/src/handlers/config/handler.tsx b/src/handlers/config/handler.tsx index a54181b10..bcf120e70 100644 --- a/src/handlers/config/handler.tsx +++ b/src/handlers/config/handler.tsx @@ -54,7 +54,7 @@ export const createConfigHandler = () => }, }); -/** Coerces a raw CLI string to the type expected by the config schema at `key`. */ +/** Coerces a raw CLI string to the type expected by the config schema at `key`. Returns original value if unable to coerce */ function coerceValue(key: string, value: string): unknown { const valueType = getGlobalConfigValueType(key); if (!valueType) { From fcd2adf9dcb10b2b0306e633429af4be0f389641 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 22 Jul 2026 23:28:10 +0000 Subject: [PATCH 13/13] fix(globalConfig): unwrap schema before returning --- src/globalConfig/schemas.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/globalConfig/schemas.tsx b/src/globalConfig/schemas.tsx index 89374ba0d..6dce86a5d 100644 --- a/src/globalConfig/schemas.tsx +++ b/src/globalConfig/schemas.tsx @@ -80,5 +80,5 @@ export function getGlobalConfigKeys(): string[] { /** Returns the base zod type name (e.g. `"boolean"`, `"string"`) for the schema at `path`, or `undefined` if the path is invalid. */ export function getGlobalConfigValueType(path: string): z.ZodType["def"]["type"] | undefined { const schema = globalConfigSchemaMap.get(path); - return schema ? schema.def.type : undefined; + return schema ? unwrapSchema(schema).def.type : undefined; }