From 53d3134fe89037074ca74bfbf2ba9309507cb275 Mon Sep 17 00:00:00 2001 From: sheeek Date: Sun, 11 Jan 2026 15:48:40 +0100 Subject: [PATCH] refactor(config): simplify includes with class-based processor - Replace free functions with IncludeProcessor class - Simplify IncludeResolver interface: { readFile, parseJson } - Break down loadFile into focused private methods - Use reduce() for array include merging - Cleaner separation of concerns --- src/config/includes.test.ts | 50 +++--- src/config/includes.ts | 296 ++++++++++++++++++------------------ src/config/io.ts | 8 +- 3 files changed, 174 insertions(+), 180 deletions(-) diff --git a/src/config/includes.test.ts b/src/config/includes.test.ts index 44a16758d..fb78d23ea 100644 --- a/src/config/includes.test.ts +++ b/src/config/includes.test.ts @@ -3,26 +3,20 @@ import { describe, expect, it } from "vitest"; import { CircularIncludeError, ConfigIncludeError, + type IncludeResolver, resolveConfigIncludes, } from "./includes.js"; -function createMockResolver(files: Record) { - const fsModule = { - readFileSync: (filePath: string) => { +function createMockResolver(files: Record): IncludeResolver { + return { + readFile: (filePath: string) => { if (filePath in files) { return JSON.stringify(files[filePath]); } - const err = new Error(`ENOENT: no such file: ${filePath}`); - (err as NodeJS.ErrnoException).code = "ENOENT"; - throw err; + throw new Error(`ENOENT: no such file: ${filePath}`); }, - } as typeof import("node:fs"); - - const json5Module = { - parse: JSON.parse, - } as typeof import("json5"); - - return { fsModule, json5Module }; + parseJson: JSON.parse, + }; } function resolve( @@ -124,9 +118,9 @@ describe("resolveConfigIncludes", () => { }); it("throws ConfigIncludeError for invalid JSON", () => { - const resolver = { - fsModule: { readFileSync: () => "{ invalid json }" } as typeof import("node:fs"), - json5Module: { parse: JSON.parse } as typeof import("json5"), + const resolver: IncludeResolver = { + readFile: () => "{ invalid json }", + parseJson: JSON.parse, }; const obj = { $include: "./bad.json" }; expect(() => resolveConfigIncludes(obj, "/config/clawdbot.json", resolver)) @@ -136,19 +130,17 @@ describe("resolveConfigIncludes", () => { }); it("throws CircularIncludeError for circular includes", () => { - const resolver = { - fsModule: { - readFileSync: (filePath: string) => { - if (filePath === "/config/a.json") { - return JSON.stringify({ $include: "./b.json" }); - } - if (filePath === "/config/b.json") { - return JSON.stringify({ $include: "./a.json" }); - } - throw new Error(`Unknown file: ${filePath}`); - }, - } as typeof import("node:fs"), - json5Module: { parse: JSON.parse } as typeof import("json5"), + const resolver: IncludeResolver = { + readFile: (filePath: string) => { + if (filePath === "/config/a.json") { + return JSON.stringify({ $include: "./b.json" }); + } + if (filePath === "/config/b.json") { + return JSON.stringify({ $include: "./a.json" }); + } + throw new Error(`Unknown file: ${filePath}`); + }, + parseJson: JSON.parse, }; const obj = { $include: "./a.json" }; expect(() => resolveConfigIncludes(obj, "/config/clawdbot.json", resolver)) diff --git a/src/config/includes.ts b/src/config/includes.ts index d7ade4c42..f3a74d332 100644 --- a/src/config/includes.ts +++ b/src/config/includes.ts @@ -1,11 +1,13 @@ /** * Config includes: $include directive for modular configs * - * Supports: - * - `{ "$include": "./path/to/file.json5" }` - single file include - * - `{ "$include": ["./a.json5", "./b.json5"] }` - deep merge multiple files - * - Nested includes up to MAX_INCLUDE_DEPTH levels - * - Circular include detection + * @example + * ```json5 + * { + * "$include": "./base.json5", // single file + * "$include": ["./a.json5", "./b.json5"] // merge multiple + * } + * ``` */ import fs from "node:fs"; @@ -13,10 +15,6 @@ import path from "node:path"; import JSON5 from "json5"; -// ============================================================================ -// Constants -// ============================================================================ - export const INCLUDE_KEY = "$include"; export const MAX_INCLUDE_DEPTH = 10; @@ -25,15 +23,8 @@ export const MAX_INCLUDE_DEPTH = 10; // ============================================================================ export type IncludeResolver = { - fsModule: typeof fs; - json5Module: typeof JSON5; -}; - -type IncludeContext = { - basePath: string; - visited: Set; - depth: number; - resolver: IncludeResolver; + readFile: (path: string) => string; + parseJson: (raw: string) => unknown; }; // ============================================================================ @@ -74,12 +65,7 @@ function isPlainObject(value: unknown): value is Record { ); } -/** - * Deep merge two values. - * - Arrays: concatenate - * - Objects: recursive merge - * - Primitives: source wins - */ +/** Deep merge: arrays concatenate, objects merge recursively, primitives: source wins */ export function deepMerge(target: unknown, source: unknown): unknown { if (Array.isArray(target) && Array.isArray(source)) { return [...target, ...source]; @@ -87,160 +73,176 @@ export function deepMerge(target: unknown, source: unknown): unknown { if (isPlainObject(target) && isPlainObject(source)) { const result: Record = { ...target }; for (const key of Object.keys(source)) { - result[key] = - key in result ? deepMerge(result[key], source[key]) : source[key]; + result[key] = key in result + ? deepMerge(result[key], source[key]) + : source[key]; } return result; } return source; } -function resolveIncludePath(includePath: string, basePath: string): string { - if (path.isAbsolute(includePath)) { - return includePath; - } - return path.resolve(path.dirname(basePath), includePath); -} - // ============================================================================ -// Core Logic +// Include Resolver Class // ============================================================================ -function loadIncludeFile(includePath: string, ctx: IncludeContext): unknown { - const resolvedPath = resolveIncludePath(includePath, ctx.basePath); - const normalizedPath = path.normalize(resolvedPath); +class IncludeProcessor { + private visited = new Set(); + private depth = 0; - if (ctx.visited.has(normalizedPath)) { - throw new CircularIncludeError([...ctx.visited, normalizedPath]); + constructor( + private basePath: string, + private resolver: IncludeResolver, + ) { + this.visited.add(path.normalize(basePath)); } - if (ctx.depth >= MAX_INCLUDE_DEPTH) { - throw new ConfigIncludeError( - `Maximum include depth (${MAX_INCLUDE_DEPTH}) exceeded at: ${includePath}`, - includePath, - ); - } - - let raw: string; - try { - raw = ctx.resolver.fsModule.readFileSync(normalizedPath, "utf-8"); - } catch (err) { - throw new ConfigIncludeError( - `Failed to read include file: ${includePath} (resolved: ${normalizedPath})`, - includePath, - err instanceof Error ? err : undefined, - ); - } - - let parsed: unknown; - try { - parsed = ctx.resolver.json5Module.parse(raw); - } catch (err) { - throw new ConfigIncludeError( - `Failed to parse include file: ${includePath} (resolved: ${normalizedPath})`, - includePath, - err instanceof Error ? err : undefined, - ); - } - - const newCtx: IncludeContext = { - ...ctx, - basePath: normalizedPath, - visited: new Set([...ctx.visited, normalizedPath]), - depth: ctx.depth + 1, - }; - - return resolveIncludesInternal(parsed, newCtx); -} - -function resolveIncludeDirective( - includeValue: unknown, - ctx: IncludeContext, -): unknown { - if (typeof includeValue === "string") { - return loadIncludeFile(includeValue, ctx); - } - - if (Array.isArray(includeValue)) { - let result: unknown = {}; - for (const item of includeValue) { - if (typeof item !== "string") { - throw new ConfigIncludeError( - `Invalid $include array item: expected string, got ${typeof item}`, - String(item), - ); - } - result = deepMerge(result, loadIncludeFile(item, ctx)); - } - return result; - } - - throw new ConfigIncludeError( - `Invalid $include value: expected string or array of strings, got ${typeof includeValue}`, - String(includeValue), - ); -} - -function resolveIncludesInternal(obj: unknown, ctx: IncludeContext): unknown { - if (Array.isArray(obj)) { - return obj.map((item) => resolveIncludesInternal(item, ctx)); - } - - if (isPlainObject(obj)) { - if (INCLUDE_KEY in obj) { - const includeValue = obj[INCLUDE_KEY]; - const otherKeys = Object.keys(obj).filter((k) => k !== INCLUDE_KEY); - - if (otherKeys.length > 0) { - const included = resolveIncludeDirective(includeValue, ctx); - const rest: Record = {}; - for (const key of otherKeys) { - rest[key] = resolveIncludesInternal(obj[key], ctx); - } - return deepMerge(included, rest); - } - - return resolveIncludeDirective(includeValue, ctx); + process(obj: unknown): unknown { + if (Array.isArray(obj)) { + return obj.map((item) => this.process(item)); } + if (!isPlainObject(obj)) { + return obj; + } + + if (!(INCLUDE_KEY in obj)) { + return this.processObject(obj); + } + + return this.processInclude(obj); + } + + private processObject(obj: Record): Record { const result: Record = {}; for (const [key, value] of Object.entries(obj)) { - result[key] = resolveIncludesInternal(value, ctx); + result[key] = this.process(value); } return result; } - return obj; + private processInclude(obj: Record): unknown { + const includeValue = obj[INCLUDE_KEY]; + const otherKeys = Object.keys(obj).filter((k) => k !== INCLUDE_KEY); + const included = this.resolveInclude(includeValue); + + if (otherKeys.length === 0) { + return included; + } + + // Merge included content with sibling keys + const rest: Record = {}; + for (const key of otherKeys) { + rest[key] = this.process(obj[key]); + } + return deepMerge(included, rest); + } + + private resolveInclude(value: unknown): unknown { + if (typeof value === "string") { + return this.loadFile(value); + } + + if (Array.isArray(value)) { + return value.reduce((merged, item) => { + if (typeof item !== "string") { + throw new ConfigIncludeError( + `Invalid $include array item: expected string, got ${typeof item}`, + String(item), + ); + } + return deepMerge(merged, this.loadFile(item)); + }, {}); + } + + throw new ConfigIncludeError( + `Invalid $include value: expected string or array of strings, got ${typeof value}`, + String(value), + ); + } + + private loadFile(includePath: string): unknown { + const resolvedPath = this.resolvePath(includePath); + + this.checkCircular(resolvedPath); + this.checkDepth(includePath); + + const raw = this.readFile(includePath, resolvedPath); + const parsed = this.parseFile(includePath, resolvedPath, raw); + + return this.processNested(resolvedPath, parsed); + } + + private resolvePath(includePath: string): string { + const resolved = path.isAbsolute(includePath) + ? includePath + : path.resolve(path.dirname(this.basePath), includePath); + return path.normalize(resolved); + } + + private checkCircular(resolvedPath: string): void { + if (this.visited.has(resolvedPath)) { + throw new CircularIncludeError([...this.visited, resolvedPath]); + } + } + + private checkDepth(includePath: string): void { + if (this.depth >= MAX_INCLUDE_DEPTH) { + throw new ConfigIncludeError( + `Maximum include depth (${MAX_INCLUDE_DEPTH}) exceeded at: ${includePath}`, + includePath, + ); + } + } + + private readFile(includePath: string, resolvedPath: string): string { + try { + return this.resolver.readFile(resolvedPath); + } catch (err) { + throw new ConfigIncludeError( + `Failed to read include file: ${includePath} (resolved: ${resolvedPath})`, + includePath, + err instanceof Error ? err : undefined, + ); + } + } + + private parseFile(includePath: string, resolvedPath: string, raw: string): unknown { + try { + return this.resolver.parseJson(raw); + } catch (err) { + throw new ConfigIncludeError( + `Failed to parse include file: ${includePath} (resolved: ${resolvedPath})`, + includePath, + err instanceof Error ? err : undefined, + ); + } + } + + private processNested(resolvedPath: string, parsed: unknown): unknown { + const nested = new IncludeProcessor(resolvedPath, this.resolver); + nested.visited = new Set([...this.visited, resolvedPath]); + nested.depth = this.depth + 1; + return nested.process(parsed); + } } // ============================================================================ // Public API // ============================================================================ +const defaultResolver: IncludeResolver = { + readFile: (p) => fs.readFileSync(p, "utf-8"), + parseJson: (raw) => JSON5.parse(raw), +}; + /** * Resolves all $include directives in a parsed config object. - * - * @param obj - Parsed config object (from JSON5.parse) - * @param configPath - Path to the main config file (for relative path resolution) - * @param resolver - Optional custom fs/json5 modules (for testing) - * @returns Config object with all includes resolved - * - * @example - * ```typescript - * const parsed = JSON5.parse(raw); - * const resolved = resolveConfigIncludes(parsed, "/path/to/config.json5"); - * ``` */ export function resolveConfigIncludes( obj: unknown, configPath: string, - resolver: IncludeResolver = { fsModule: fs, json5Module: JSON5 }, + resolver: IncludeResolver = defaultResolver, ): unknown { - const ctx: IncludeContext = { - basePath: configPath, - visited: new Set([path.normalize(configPath)]), - depth: 0, - resolver, - }; - return resolveIncludesInternal(obj, ctx); + return new IncludeProcessor(configPath, resolver).process(obj); } diff --git a/src/config/io.ts b/src/config/io.ts index 3d7326f8f..f05f9f19f 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -162,8 +162,8 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { // Resolve $include directives before validation const resolved = resolveConfigIncludes(parsed, configPath, { - fsModule: deps.fs, - json5Module: deps.json5, + readFile: (p) => deps.fs.readFileSync(p, "utf-8"), + parseJson: (raw) => deps.json5.parse(raw), }); warnOnConfigMiskeys(resolved, deps.logger); @@ -267,8 +267,8 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { let resolved: unknown; try { resolved = resolveConfigIncludes(parsedRes.parsed, configPath, { - fsModule: deps.fs, - json5Module: deps.json5, + readFile: (p) => deps.fs.readFileSync(p, "utf-8"), + parseJson: (raw) => deps.json5.parse(raw), }); } catch (err) { const message =