refactor(parsing): move parsing back down into router

Author: Hweinstock

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");
-
       cachedConfig = await config.source.write(newConfig);
-
       return cachedConfig;
     },
   };

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 } from "./schemas";

src/globalConfig/schemas.tsx

@@ -0,0 +1,35 @@
+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 = <TShape extends z.ZodRawShape>(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.
+ * 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: coercedBoolean.optional(),
+    endpoint: z.string().optional(),
+  }).optional(),
+  installationId: z.uuid().optional(),
+});

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<TSchema> {
+export interface JsonFileDataSourceConfig<TSchema extends z.ZodObject> {
   filePath: string;
   /**
    * Describes the data to write to the file if it does not exist or is deleted.
    */
-  initialData: z.infer<TSchema>;
+  getDefaultData(): z.infer<TSchema>;
   schema: TSchema;
   logger: Logger;
 }
@@ -58,7 +57,7 @@ export function createJsonFileDataSource<TSchema extends z.ZodObject>(
         // 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`);
-          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<TSchema extends z.ZodObject>(
           })
           .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;

src/globalConfig/types.tsx

@@ -1,31 +1,5 @@
-import z from "zod";
-
-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 = <TShape extends z.ZodRawShape>(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(),
-});
+import type z from "zod";
+import type { globalConfigSchema } from "./schemas";
 
 /**
  * Shape of the global config built from {@link globalConfigSchema}
@@ -34,13 +8,16 @@ export type GlobalConfig = z.infer<typeof globalConfigSchema>;
 
 /**
  * Interface for setting and retrieving information from the global configuration for the CLI.
- * ex. telemetry settings.
+ * ex. telemetry settings, endpoint overrides, etc.
  */
 export interface GlobalConfigAccessor {
   get(): Promise<GlobalConfig>;
   set(newConfig: GlobalConfig): Promise<GlobalConfig>;
 }
 
+/**
+ * Generic interface for the source of a typed object
+ */
 export interface JsonDataSource<TShape> {
   read(): Promise<TShape>;
   write(data: TShape): Promise<TShape>;

src/handlers/config/config.test.tsx

@@ -41,7 +41,7 @@ describe("config", () => {
       source: createJsonFileDataSource({
         filePath: configPath,
         schema: globalConfigSchema,
-        initialData: initialConfig,
+        getDefaultData: () => initialConfig,
         logger,
       }),
     });
@@ -122,17 +122,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");
+    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({
@@ -160,5 +160,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);
   });
 });

src/handlers/config/handler.tsx

@@ -1,6 +1,5 @@
 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";
 
@@ -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();
@@ -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<string, unknown> {
   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<unknown>((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<string, unknown>,
   path: string,
@@ -80,6 +80,7 @@ function setAtPath(
   };
 }
 
+/** Checks whether `path` corresponds to a key recognized by {@link globalConfigSchema}. */
 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.

src/handlers/index.tsx

@@ -4,7 +4,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";
@@ -34,7 +34,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));

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" }),
         }),
       });

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";