From 68ba7ccae6c9247e96d81e4eb7e38be827149377 Mon Sep 17 00:00:00 2001 From: Stefano Maffeis Date: Tue, 21 Jul 2026 19:18:45 +0200 Subject: [PATCH] Fail loudly on invalid config files (#9041) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server config loader reads `.paperclip/config.json` and feeds it into the shared Paperclip config schema. > - When a config file exists but cannot be parsed or fails schema validation, Paperclip should not silently ignore it. > - The current `readConfigFile()` catch block treats invalid files the same as missing files, so startup falls back to defaults while the banner can still point at the ignored config path. > - This pull request keeps the missing-file fallback, but makes present invalid config files fail with a path-specific error. > - The benefit is safer startup behavior and a clear diagnostic that points at the invalid config field. ## Linked Issues or Issue Description Fixes #8908 ## What Changed - Changed `readConfigFile()` to return `null` only when the config file is absent. - Added explicit errors for unreadable/invalid JSON config files. - Added explicit Zod validation errors that include the config path and invalid field path without printing config contents. - Added server tests for missing config, invalid JSON, schema validation failure, and valid config parsing. ## Verification - `pnpm exec vitest run server/src/__tests__/config-file.test.ts` - `pnpm --filter @paperclipai/server typecheck` ## Risks Low risk for valid configs and missing configs. This intentionally changes behavior for present invalid config files from silent fallback to startup failure, which is the issue being fixed. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex based on GPT-5, with repository file inspection, GitHub CLI, and local command execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- server/src/__tests__/config-file.test.ts | 92 ++++++++++++++++++++++++ server/src/config-file.ts | 27 ++++++- 2 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 server/src/__tests__/config-file.test.ts diff --git a/server/src/__tests__/config-file.test.ts b/server/src/__tests__/config-file.test.ts new file mode 100644 index 0000000000..9d5b2701ca --- /dev/null +++ b/server/src/__tests__/config-file.test.ts @@ -0,0 +1,92 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { readConfigFile } from "../config-file.js"; + +const ORIGINAL_PAPERCLIP_CONFIG = process.env.PAPERCLIP_CONFIG; + +function writeConfig(configPath: string, value: unknown): void { + fs.writeFileSync(configPath, `${JSON.stringify(value, null, 2)}\n`); +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +function minimalConfig(): unknown { + return { + $meta: { + version: 1, + updatedAt: "2026-07-05T00:00:00.000Z", + source: "configure", + }, + database: { + mode: "embedded-postgres", + }, + logging: { + mode: "file", + }, + server: {}, + }; +} + +describe("readConfigFile", () => { + let tempDir: string; + let configPath: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-config-file-test-")); + configPath = path.join(tempDir, "config.json"); + process.env.PAPERCLIP_CONFIG = configPath; + }); + + afterEach(() => { + if (ORIGINAL_PAPERCLIP_CONFIG === undefined) { + delete process.env.PAPERCLIP_CONFIG; + } else { + process.env.PAPERCLIP_CONFIG = ORIGINAL_PAPERCLIP_CONFIG; + } + + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it("returns null when the config file does not exist", () => { + expect(readConfigFile()).toBeNull(); + }); + + it("throws a path-specific error when the config file is invalid JSON", () => { + fs.writeFileSync(configPath, "{"); + + expect(() => readConfigFile()).toThrow( + new RegExp(`Invalid Paperclip config at ${escapeRegExp(configPath)}: failed to read or parse JSON`), + ); + }); + + it("throws a field-specific error when the config file fails schema validation", () => { + const config = minimalConfig(); + if (typeof config === "object" && config !== null) { + (config as { $meta: { source: string } }).$meta.source = "edited-by-hand"; + } + + writeConfig(configPath, config); + + expect(() => readConfigFile()).toThrow(/Invalid Paperclip config .* \$meta\.source:/); + }); + + it("parses a valid config file", () => { + writeConfig(configPath, minimalConfig()); + + expect(readConfigFile()).toMatchObject({ + $meta: { + source: "configure", + }, + database: { + mode: "embedded-postgres", + }, + logging: { + mode: "file", + }, + }); + }); +}); diff --git a/server/src/config-file.ts b/server/src/config-file.ts index a25d4db58c..9c150f4a78 100644 --- a/server/src/config-file.ts +++ b/server/src/config-file.ts @@ -1,16 +1,37 @@ import fs from "node:fs"; import { paperclipConfigSchema, type PaperclipConfig } from "@paperclipai/shared"; +import { ZodError } from "zod"; import { resolvePaperclipConfigPath } from "./paths.js"; +function formatConfigValidationError(error: ZodError): string { + return error.issues + .map((issue) => { + const issuePath = issue.path.length > 0 ? issue.path.join(".") : ""; + return `${issuePath}: ${issue.message}`; + }) + .join("; "); +} + export function readConfigFile(): PaperclipConfig | null { const configPath = resolvePaperclipConfigPath(); if (!fs.existsSync(configPath)) return null; + let raw: unknown; + try { + raw = JSON.parse(fs.readFileSync(configPath, "utf-8")); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new Error(`Invalid Paperclip config at ${configPath}: failed to read or parse JSON: ${reason}`); + } + try { - const raw = JSON.parse(fs.readFileSync(configPath, "utf-8")); return paperclipConfigSchema.parse(raw); - } catch { - return null; + } catch (error) { + if (error instanceof ZodError) { + throw new Error(`Invalid Paperclip config at ${configPath}: ${formatConfigValidationError(error)}`); + } + + throw error; } }