diff --git a/src/config/loader.ts b/src/config/loader.ts index 140c3a1fafd8092f5f72fa95c313f404a1da1f58..b4a55559e2a831ba12a8c1ce16aa0e07fbf0b312 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -1,6 +1,8 @@ import { readFile } from "node:fs/promises"; import { resolve } from "node:path"; +import { Value } from "@sinclair/typebox/value"; import { defaultConfig } from "./defaults.js"; +import { ConfigSchema, PartialConfigSchema } from "./schema.js"; import type { RumiloConfig } from "./schema.js"; import { ConfigError } from "../util/errors.js"; import toml from "toml"; @@ -31,12 +33,27 @@ function mergeConfig(base: RumiloConfig, override: Partial): Rumil }; } -function validateConfig(config: RumiloConfig): void { - if (!config.defaults.model) { - throw new ConfigError("defaults.model is required"); +function validatePartialConfig(parsed: unknown): asserts parsed is Partial { + if (!Value.Check(PartialConfigSchema, parsed)) { + const errors = [...Value.Errors(PartialConfigSchema, parsed)]; + const details = errors + .map((e) => ` ${e.path}: ${e.message} (got ${JSON.stringify(e.value)})`) + .join("\n"); + throw new ConfigError( + `Invalid config:\n${details}`, + ); } - if (typeof config.defaults.cleanup !== "boolean") { - throw new ConfigError("defaults.cleanup must be a boolean"); +} + +function validateFullConfig(config: unknown): asserts config is RumiloConfig { + if (!Value.Check(ConfigSchema, config)) { + const errors = [...Value.Errors(ConfigSchema, config)]; + const details = errors + .map((e) => ` ${e.path}: ${e.message} (got ${JSON.stringify(e.value)})`) + .join("\n"); + throw new ConfigError( + `Invalid merged config:\n${details}`, + ); } } @@ -47,16 +64,21 @@ export async function loadConfig(): Promise { try { const raw = await readFile(configPath, "utf8"); - const parsed = toml.parse(raw) as Partial; + const parsed: unknown = toml.parse(raw); + validatePartialConfig(parsed); const merged = mergeConfig(base, parsed); - validateConfig(merged); + validateFullConfig(merged); return { config: merged, path: configPath }; } catch (error: any) { if (error?.code === "ENOENT") { - validateConfig(base); + validateFullConfig(base); return { config: base }; } + if (error instanceof ConfigError) { + throw error; + } + if (error instanceof Error) { throw new ConfigError(error.message); } @@ -70,6 +92,6 @@ export function applyConfigOverrides( overrides: Partial, ): RumiloConfig { const merged = mergeConfig(config, overrides); - validateConfig(merged); + validateFullConfig(merged); return merged; } diff --git a/src/config/schema.ts b/src/config/schema.ts index 574cb854d957fcc64a6c095cdb20e0644eb09d83..4adafc605438b9b77f26298e1c9324b20ab912f0 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -1,4 +1,4 @@ -import { Type, type Static } from "@sinclair/typebox"; +import { Type, type Static, type TObject, type TProperties } from "@sinclair/typebox"; const CustomModelSchema = Type.Object({ provider: Type.String(), @@ -33,7 +33,7 @@ const CustomModelSchema = Type.Object({ ), }); -const ConfigSchema = Type.Object({ +export const ConfigSchema = Type.Object({ defaults: Type.Object({ model: Type.String(), cleanup: Type.Boolean(), @@ -55,5 +55,21 @@ const ConfigSchema = Type.Object({ custom_models: Type.Optional(Type.Record(Type.String(), CustomModelSchema)), }); +/** Deep-partial version of ConfigSchema for validating TOML override files. */ +function partialObject(schema: TObject) { + const partial: Record = {}; + for (const [key, value] of Object.entries(schema.properties)) { + partial[key] = Type.Optional(value as any); + } + return Type.Object(partial as any); +} + +export const PartialConfigSchema = Type.Object({ + defaults: Type.Optional(partialObject(ConfigSchema.properties.defaults)), + web: Type.Optional(partialObject(ConfigSchema.properties.web)), + repo: Type.Optional(partialObject(ConfigSchema.properties.repo)), + custom_models: Type.Optional(Type.Record(Type.String(), CustomModelSchema)), +}); + export type RumiloConfig = Static; export type CustomModelConfig = Static; diff --git a/test/config-validation.test.ts b/test/config-validation.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..1da4518d18f8ff7fb4c0c99b26407c77a4d4655f --- /dev/null +++ b/test/config-validation.test.ts @@ -0,0 +1,128 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { ConfigError } from "../src/util/errors.js"; +import { loadConfig } from "../src/config/loader.js"; + +describe("config validation", () => { + let configDir: string; + let configPath: string; + const originalEnv = { ...process.env }; + + beforeEach(() => { + configDir = mkdtempSync(join(tmpdir(), "rumilo-cfg-test-")); + configPath = join(configDir, "config.toml"); + process.env["XDG_CONFIG_HOME"] = join(configDir, ".."); + // loadConfig looks for /rumilo/config.toml + // So we need the dir structure to match + const rumiloDir = join(configDir, "..", "rumilo"); + require("node:fs").mkdirSync(rumiloDir, { recursive: true }); + configPath = join(rumiloDir, "config.toml"); + }); + + afterEach(() => { + process.env = { ...originalEnv }; + try { + rmSync(configDir, { recursive: true, force: true }); + // Also clean up the rumilo dir we created + const rumiloDir = join(configDir, "..", "rumilo"); + rmSync(rumiloDir, { recursive: true, force: true }); + } catch {} + }); + + test("rejects defaults.model with wrong type (number instead of string)", async () => { + writeFileSync( + configPath, + `[defaults]\nmodel = 42\ncleanup = true\n`, + ); + await expect(loadConfig()).rejects.toThrow(ConfigError); + await expect(loadConfig()).rejects.toThrow(/defaults\/model/); + }); + + test("rejects defaults.cleanup with wrong type (string instead of boolean)", async () => { + writeFileSync( + configPath, + `[defaults]\nmodel = "anthropic:claude-sonnet-4-20250514"\ncleanup = "yes"\n`, + ); + await expect(loadConfig()).rejects.toThrow(ConfigError); + await expect(loadConfig()).rejects.toThrow(/defaults\/cleanup/); + }); + + test("rejects repo.default_depth with wrong type (string instead of number)", async () => { + writeFileSync( + configPath, + `[repo]\ndefault_depth = "deep"\n`, + ); + await expect(loadConfig()).rejects.toThrow(ConfigError); + await expect(loadConfig()).rejects.toThrow(/repo\/default_depth/); + }); + + test("rejects repo.default_depth below minimum (0)", async () => { + writeFileSync( + configPath, + `[repo]\ndefault_depth = 0\n`, + ); + await expect(loadConfig()).rejects.toThrow(ConfigError); + await expect(loadConfig()).rejects.toThrow(/default_depth/); + }); + + test("rejects unknown top-level section type (number instead of object)", async () => { + // web should be an object but we pass a string value at top level + writeFileSync( + configPath, + `[defaults]\nmodel = "x"\ncleanup = true\n[web]\nmodel = 123\n`, + ); + await expect(loadConfig()).rejects.toThrow(ConfigError); + }); + + test("accepts valid partial config (only [repo] section)", async () => { + writeFileSync( + configPath, + `[repo]\nmodel = "anthropic:claude-sonnet-4-20250514"\ndefault_depth = 5\n`, + ); + const { config } = await loadConfig(); + expect(config.repo.model).toBe("anthropic:claude-sonnet-4-20250514"); + expect(config.repo.default_depth).toBe(5); + // defaults should come from defaultConfig + expect(config.defaults.model).toBe("anthropic:claude-sonnet-4-20250514"); + }); + + test("accepts valid complete config", async () => { + writeFileSync( + configPath, + [ + `[defaults]`, + `model = "openai:gpt-4"`, + `cleanup = false`, + ``, + `[web]`, + `model = "openai:gpt-4"`, + ``, + `[repo]`, + `model = "openai:gpt-4"`, + `default_depth = 3`, + `blob_limit = "10m"`, + ].join("\n"), + ); + const { config } = await loadConfig(); + expect(config.defaults.model).toBe("openai:gpt-4"); + expect(config.defaults.cleanup).toBe(false); + expect(config.repo.default_depth).toBe(3); + }); + + test("error message includes path and expected type for diagnostics", async () => { + writeFileSync( + configPath, + `[defaults]\nmodel = 42\ncleanup = true\n`, + ); + try { + await loadConfig(); + throw new Error("should have thrown"); + } catch (e: any) { + expect(e).toBeInstanceOf(ConfigError); + expect(e.message).toContain("/defaults/model"); + expect(e.message).toMatch(/string/i); + } + }); +});