From 3097d96581f36a52049807b105ec81fd15c6d3eb Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 6 Aug 2026 12:46:57 +0000 Subject: [PATCH] fix(config): preserve extensions and guard invalid repairs Co-Authored-By: Paperclip --- cli/src/__tests__/config-store.test.ts | 153 ++++++++++++++++++ cli/src/__tests__/configure.test.ts | 17 ++ .../__tests__/invalid-config-repair.test.ts | 123 ++++++++++++++ cli/src/__tests__/onboard.test.ts | 16 ++ cli/src/commands/configure.ts | 53 ++++-- cli/src/commands/onboard.ts | 45 +++++- cli/src/config/schema.ts | 3 + cli/src/config/store.ts | 118 +++++++++++++- doc/DEVELOPING.md | 4 + packages/shared/src/config-persistence.ts | 27 ++++ packages/shared/src/config-schema.test.ts | 69 ++++++-- packages/shared/src/config-schema.ts | 117 ++++++++++++-- server/src/__tests__/config-file.test.ts | 17 +- server/src/__tests__/worktree-config.test.ts | 47 ++++++ server/src/config-file.ts | 12 +- server/src/worktree-config.ts | 38 ++++- 16 files changed, 798 insertions(+), 61 deletions(-) create mode 100644 cli/src/__tests__/config-store.test.ts create mode 100644 cli/src/__tests__/invalid-config-repair.test.ts create mode 100644 packages/shared/src/config-persistence.ts diff --git a/cli/src/__tests__/config-store.test.ts b/cli/src/__tests__/config-store.test.ts new file mode 100644 index 00000000000..600edb36d9b --- /dev/null +++ b/cli/src/__tests__/config-store.test.ts @@ -0,0 +1,153 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + backupInvalidConfig, + readConfig, + writeConfig, +} from "../config/store.js"; +import type { PaperclipConfig } from "../config/schema.js"; + +const roots: string[] = []; + +afterEach(() => { + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +function configFixture(): PaperclipConfig { + return { + $meta: { + version: 1, + updatedAt: "2026-08-06T00:00:00.000Z", + source: "configure", + }, + database: { + mode: "embedded-postgres", + embeddedPostgresDataDir: "/tmp/paperclip-db", + embeddedPostgresPort: 54329, + backup: { + enabled: true, + intervalMinutes: 60, + retentionDays: 30, + dir: "/tmp/paperclip-backups", + }, + }, + logging: { mode: "file", logDir: "/tmp/paperclip-logs" }, + server: { + deploymentMode: "local_trusted", + exposure: "private", + bind: "loopback", + host: "127.0.0.1", + port: 3100, + allowedHostnames: [], + serveUi: true, + }, + auth: { baseUrlMode: "auto", disableSignUp: false }, + telemetry: { enabled: true }, + storage: { + provider: "local_disk", + localDisk: { baseDir: "/tmp/paperclip-storage" }, + s3: { + bucket: "paperclip", + region: "us-east-1", + prefix: "", + forcePathStyle: false, + }, + }, + secrets: { + provider: "local_encrypted", + strictMode: false, + localEncrypted: { keyFilePath: "/tmp/paperclip-secrets/master.key" }, + }, + }; +} + +function tempConfigPath(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-config-store-")); + roots.push(root); + return path.join(root, "config.json"); +} + +describe("config store", () => { + it("preserves unknown top-level and nested keys through a known-field update", () => { + const configPath = tempConfigPath(); + const raw = { + ...configFixture(), + futurePlugin: { enabled: true }, + storage: { + ...configFixture().storage, + localDisk: { + ...configFixture().storage.localDisk, + futureMountOption: "cached", + }, + }, + }; + fs.writeFileSync(configPath, `${JSON.stringify(raw, null, 2)}\n`); + + const config = readConfig(configPath)!; + config.server.port = 3200; + expect(writeConfig(config, configPath)).toBe(true); + + const written = JSON.parse(fs.readFileSync(configPath, "utf8")); + expect(written.server.port).toBe(3200); + expect(written.futurePlugin).toEqual({ enabled: true }); + expect(written.storage.localDisk.futureMountOption).toBe("cached"); + }); + + it("skips semantically unchanged writes and keeps the original mtime and bytes", () => { + const configPath = tempConfigPath(); + const contents = `${JSON.stringify(configFixture())}\n`; + fs.writeFileSync(configPath, contents); + const stableTime = new Date("2026-01-01T00:00:00.000Z"); + fs.utimesSync(configPath, stableTime, stableTime); + + const config = readConfig(configPath)!; + config.$meta.updatedAt = new Date().toISOString(); + config.$meta.source = "configure"; + + expect(writeConfig(config, configPath)).toBe(false); + expect(fs.readFileSync(configPath, "utf8")).toBe(contents); + expect(fs.statSync(configPath).mtimeMs).toBe(stableTime.getTime()); + expect(fs.existsSync(`${configPath}.backup`)).toBe(false); + }); + + it("removes an explicitly cleared owned optional section", () => { + const configPath = tempConfigPath(); + const configWithLlm: PaperclipConfig = { + ...configFixture(), + llm: { provider: "openai", futureProviderOption: true }, + }; + fs.writeFileSync(configPath, `${JSON.stringify(configWithLlm, null, 2)}\n`); + + const config = readConfig(configPath)!; + config.llm = undefined; + + expect(writeConfig(config, configPath)).toBe(true); + expect(JSON.parse(fs.readFileSync(configPath, "utf8"))).not.toHaveProperty("llm"); + }); + + it("creates collision-safe byte-for-byte invalid backups", () => { + const configPath = tempConfigPath(); + const invalidBytes = Buffer.from([0x7b, 0x22, 0x78, 0x22, 0x3a, 0xff, 0x7d]); + fs.writeFileSync(configPath, invalidBytes); + + const first = backupInvalidConfig(configPath); + const second = backupInvalidConfig(configPath); + + expect(first).toBe(`${configPath}.invalid-1`); + expect(second).toBe(`${configPath}.invalid-2`); + expect(fs.readFileSync(first)).toEqual(invalidBytes); + expect(fs.readFileSync(second)).toEqual(invalidBytes); + }); + + it("refuses to overwrite an invalid config without explicit repair authorization", () => { + const configPath = tempConfigPath(); + fs.writeFileSync(configPath, "{ invalid"); + + expect(() => writeConfig(configFixture(), configPath)).toThrow(/Refusing to overwrite invalid config/); + expect(fs.readFileSync(configPath, "utf8")).toBe("{ invalid"); + }); +}); diff --git a/cli/src/__tests__/configure.test.ts b/cli/src/__tests__/configure.test.ts index 74a37fc8fcc..bf25d0d79a1 100644 --- a/cli/src/__tests__/configure.test.ts +++ b/cli/src/__tests__/configure.test.ts @@ -96,4 +96,21 @@ describe("configure command", () => { fs.rmSync(root, { recursive: true, force: true }); } }); + + it("backs up an invalid config and preserves it in non-interactive mode", async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-configure-invalid-")); + const configPath = path.join(root, "config.json"); + const invalidContents = "{\n invalid: true\n}\n"; + fs.writeFileSync(configPath, invalidContents); + + try { + await configure({ config: configPath, section: "server" }); + + expect(process.exitCode).toBe(1); + expect(fs.readFileSync(configPath, "utf8")).toBe(invalidContents); + expect(fs.readFileSync(`${configPath}.invalid-1`, "utf8")).toBe(invalidContents); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); }); diff --git a/cli/src/__tests__/invalid-config-repair.test.ts b/cli/src/__tests__/invalid-config-repair.test.ts new file mode 100644 index 00000000000..627002dfb95 --- /dev/null +++ b/cli/src/__tests__/invalid-config-repair.test.ts @@ -0,0 +1,123 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import * as prompts from "@clack/prompts"; +import { configure } from "../commands/configure.js"; +import { onboard } from "../commands/onboard.js"; +import { paperclipConfigSchema } from "../config/schema.js"; + +vi.mock("@clack/prompts", () => ({ + intro: vi.fn(), + outro: vi.fn(), + cancel: vi.fn(), + confirm: vi.fn(), + select: vi.fn(), + isCancel: vi.fn(() => false), + note: vi.fn(), + log: { + error: vi.fn(), + info: vi.fn(), + message: vi.fn(), + step: vi.fn(), + success: vi.fn(), + warn: vi.fn(), + }, + spinner: vi.fn(() => ({ start: vi.fn(), stop: vi.fn() })), +})); + +vi.mock("../prompts/logging.js", () => ({ + promptLogging: vi.fn(async () => ({ mode: "file", logDir: "/tmp/repaired-logs" })), +})); + +vi.mock("../onboard-service.js", () => ({ + handleOnboardService: vi.fn(async () => false), +})); + +const ORIGINAL_ENV = { ...process.env }; +const ORIGINAL_EXIT_CODE = process.exitCode; +const ORIGINAL_STDIN_IS_TTY = process.stdin.isTTY; +const ORIGINAL_STDOUT_IS_TTY = process.stdout.isTTY; +const roots: string[] = []; + +function invalidConfigFixture(): { root: string; configPath: string; contents: string } { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-invalid-config-repair-")); + roots.push(root); + const configPath = path.join(root, "config.json"); + const contents = "{\n invalid: true\n}\n"; + fs.writeFileSync(configPath, contents); + return { root, configPath, contents }; +} + +describe("invalid config repair", () => { + beforeEach(() => { + process.env = { ...ORIGINAL_ENV }; + process.exitCode = undefined; + Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); + Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); + vi.clearAllMocks(); + }); + + afterEach(() => { + process.env = { ...ORIGINAL_ENV }; + process.exitCode = ORIGINAL_EXIT_CODE; + Object.defineProperty(process.stdin, "isTTY", { value: ORIGINAL_STDIN_IS_TTY, configurable: true }); + Object.defineProperty(process.stdout, "isTTY", { value: ORIGINAL_STDOUT_IS_TTY, configurable: true }); + vi.restoreAllMocks(); + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("keeps the original and backup when interactive configure repair is declined", async () => { + const fixture = invalidConfigFixture(); + vi.mocked(prompts.confirm).mockResolvedValue(false); + + await configure({ config: fixture.configPath, section: "logging" }); + + expect(process.exitCode).toBe(1); + expect(fs.readFileSync(fixture.configPath, "utf8")).toBe(fixture.contents); + expect(fs.readFileSync(`${fixture.configPath}.invalid-1`, "utf8")).toBe(fixture.contents); + expect(prompts.cancel).toHaveBeenCalledWith(expect.stringContaining(`${fixture.configPath}.invalid-1`)); + }); + + it("atomically repairs configure from defaults after explicit confirmation", async () => { + const fixture = invalidConfigFixture(); + vi.mocked(prompts.confirm).mockResolvedValue(true); + const renameSpy = vi.spyOn(fs, "renameSync"); + + await configure({ config: fixture.configPath, section: "logging" }); + + const repaired = JSON.parse(fs.readFileSync(fixture.configPath, "utf8")); + expect(repaired.logging).toEqual({ mode: "file", logDir: "/tmp/repaired-logs" }); + expect(fs.readFileSync(`${fixture.configPath}.invalid-1`, "utf8")).toBe(fixture.contents); + expect(renameSpy).toHaveBeenCalledWith( + expect.stringMatching(/\.config\.json\.tmp-/), + fixture.configPath, + ); + }); + + it("prompts and atomically repairs invalid config during interactive onboarding", async () => { + const fixture = invalidConfigFixture(); + process.env.PAPERCLIP_HOME = fixture.root; + delete process.env.PAPERCLIP_AGENT_JWT_SECRET; + delete process.env.PAPERCLIP_SECRETS_MASTER_KEY; + delete process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE; + vi.mocked(prompts.confirm) + .mockResolvedValueOnce(true) + .mockResolvedValueOnce(false); + vi.mocked(prompts.select).mockResolvedValue("quickstart"); + const renameSpy = vi.spyOn(fs, "renameSync"); + + await onboard({ config: fixture.configPath }); + + const repaired = JSON.parse(fs.readFileSync(fixture.configPath, "utf8")); + expect(repaired.$meta.source).toBe("onboard"); + expect(() => paperclipConfigSchema.parse(repaired)).not.toThrow(); + expect(fs.readFileSync(`${fixture.configPath}.invalid-1`, "utf8")).toBe(fixture.contents); + expect(renameSpy).toHaveBeenCalledWith( + expect.stringMatching(/\.config\.json\.tmp-/), + fixture.configPath, + ); + }); +}); diff --git a/cli/src/__tests__/onboard.test.ts b/cli/src/__tests__/onboard.test.ts index 33890e703de..996444681bd 100644 --- a/cli/src/__tests__/onboard.test.ts +++ b/cli/src/__tests__/onboard.test.ts @@ -8,6 +8,7 @@ import type { PaperclipConfig } from "../config/schema.js"; const ORIGINAL_ENV = { ...process.env }; const ORIGINAL_CWD = process.cwd(); const ORIGINAL_PATH = process.env.PATH; +const ORIGINAL_EXIT_CODE = process.exitCode; function createExistingConfigFixture() { const root = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-onboard-")); @@ -106,6 +107,7 @@ describe("onboard", () => { afterEach(() => { process.env = { ...ORIGINAL_ENV }; + process.exitCode = ORIGINAL_EXIT_CODE; process.chdir(ORIGINAL_CWD); }); @@ -129,6 +131,20 @@ describe("onboard", () => { expect(fs.existsSync(path.join(path.dirname(fixture.configPath), ".env"))).toBe(true); }); + it("backs up an invalid config and preserves it during non-interactive onboarding", async () => { + const configPath = createFreshConfigPath(); + const invalidContents = Buffer.from("{\n invalid: true\n}\n"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, invalidContents); + + await onboard({ config: configPath, yes: true, invokedByRun: true }); + + expect(process.exitCode).toBe(1); + expect(fs.readFileSync(configPath)).toEqual(invalidContents); + expect(fs.readFileSync(`${configPath}.invalid-1`)).toEqual(invalidContents); + expect(fs.existsSync(path.join(path.dirname(configPath), ".env"))).toBe(false); + }); + it("keeps --yes onboarding on local trusted loopback defaults", async () => { const configPath = createFreshConfigPath(); process.env.HOST = "0.0.0.0"; diff --git a/cli/src/commands/configure.ts b/cli/src/commands/configure.ts index 7c9b391bf0f..c6b2aa9cb6a 100644 --- a/cli/src/commands/configure.ts +++ b/cli/src/commands/configure.ts @@ -1,6 +1,11 @@ import * as p from "@clack/prompts"; import pc from "picocolors"; -import { readConfig, writeConfig, configExists, resolveConfigPath } from "../config/store.js"; +import { + backupInvalidConfig, + readConfigState, + writeConfig, + resolveConfigPath, +} from "../config/store.js"; import type { PaperclipConfig } from "../config/schema.js"; import { ensureLocalSecretsKeyFile } from "../config/secrets-key.js"; import { promptDatabase } from "../prompts/database.js"; @@ -80,7 +85,8 @@ export async function configure(opts: { p.intro(pc.bgCyan(pc.black(" paperclip configure "))); const configPath = resolveConfigPath(opts.config); - if (!configExists(opts.config)) { + const configState = readConfigState(opts.config); + if (configState.status === "missing") { p.log.error("No config file found. Run `paperclipai onboard` first."); p.outro(""); process.exitCode = 1; @@ -88,15 +94,39 @@ export async function configure(opts: { } let config: PaperclipConfig; - try { - config = readConfig(opts.config) ?? defaultConfig(); - } catch (err) { + let allowInvalidReplace = false; + if (configState.status === "valid") { + config = configState.config; + } else { + const backupPath = backupInvalidConfig(opts.config); p.log.message( pc.yellow( - `Existing config is invalid. Loading defaults so you can repair it now.\n${err instanceof Error ? err.message : String(err)}`, + `Existing config is invalid. Preserved a byte-for-byte backup at ${backupPath}.\n${configState.error.message}`, ), ); + + const interactive = process.stdin.isTTY === true && process.stdout.isTTY === true; + if (!interactive) { + p.log.error( + `Refusing to replace invalid config in non-interactive mode. Fix ${configState.path} manually or rerun configure in an interactive terminal to repair from defaults.`, + ); + p.outro(""); + process.exitCode = 1; + return; + } + + const repair = await p.confirm({ + message: `Repair from defaults? This replaces ${configState.path}; the invalid original is preserved at ${backupPath}.`, + initialValue: false, + }); + if (p.isCancel(repair) || !repair) { + p.cancel(`Configuration repair aborted. Original and backup remain at ${configState.path} and ${backupPath}.`); + process.exitCode = 1; + return; + } + config = defaultConfig(); + allowInvalidReplace = true; } let section: Section | undefined = opts.section as Section | undefined; @@ -139,7 +169,7 @@ export async function configure(opts: { if (llm) { config.llm = llm; } else { - delete config.llm; + config.llm = undefined; } break; } @@ -179,8 +209,13 @@ export async function configure(opts: { config.$meta.updatedAt = new Date().toISOString(); config.$meta.source = "configure"; - writeConfig(config, opts.config); - p.log.success(`${SECTION_LABELS[section]} configuration updated.`); + const wroteConfig = writeConfig(config, opts.config, { allowInvalidReplace }); + allowInvalidReplace = false; + if (wroteConfig) { + p.log.success(`${SECTION_LABELS[section]} configuration updated.`); + } else { + p.log.message(`${SECTION_LABELS[section]} configuration unchanged; skipped writing config.json.`); + } // If section was provided via CLI flag, don't loop if (opts.section) { diff --git a/cli/src/commands/onboard.ts b/cli/src/commands/onboard.ts index 3024360dda5..eabc62c1f74 100644 --- a/cli/src/commands/onboard.ts +++ b/cli/src/commands/onboard.ts @@ -17,7 +17,12 @@ import { type SecretProvider, type StorageProvider, } from "@paperclipai/shared"; -import { configExists, readConfig, resolveConfigPath, writeConfig } from "../config/store.js"; +import { + backupInvalidConfig, + readConfigState, + resolveConfigPath, + writeConfig, +} from "../config/store.js"; import type { PaperclipConfig } from "../config/schema.js"; import { ensureAgentJwtSecret, resolveAgentJwtEnvFile } from "../config/env.js"; import { ensureLocalSecretsKeyFile } from "../config/secrets-key.js"; @@ -356,17 +361,43 @@ export async function onboard(opts: OnboardOptions): Promise { ); let existingConfig: PaperclipConfig | null = null; - if (configExists(opts.config)) { + let allowInvalidReplace = false; + const configState = readConfigState(opts.config); + if (configState.status !== "missing") { p.log.message(pc.dim(`${configPath} exists`)); - try { - existingConfig = readConfig(opts.config); - } catch (err) { + if (configState.status === "valid") { + existingConfig = configState.config; + } else { + const backupPath = backupInvalidConfig(opts.config); p.log.message( pc.yellow( - `Existing config appears invalid and will be updated.\n${err instanceof Error ? err.message : String(err)}`, + `Existing config is invalid. Preserved a byte-for-byte backup at ${backupPath}.\n${configState.error.message}`, ), ); + + const interactive = opts.yes !== true && opts.invokedByRun !== true + && process.stdin.isTTY === true && process.stdout.isTTY === true; + if (!interactive) { + p.log.error( + `Refusing to replace invalid config in non-interactive mode. Fix ${configPath} manually or rerun onboard in an interactive terminal to repair from defaults.`, + ); + p.outro(""); + process.exitCode = 1; + return; + } + + const repair = await p.confirm({ + message: `Repair from defaults? This replaces ${configPath}; the invalid original is preserved at ${backupPath}.`, + initialValue: false, + }); + if (p.isCancel(repair) || !repair) { + p.cancel(`Configuration repair aborted. Original and backup remain at ${configPath} and ${backupPath}.`); + process.exitCode = 1; + return; + } + + allowInvalidReplace = true; } } @@ -646,7 +677,7 @@ export async function onboard(opts: OnboardOptions): Promise { p.log.message(pc.dim(`Using existing local secrets key file at ${keyResult.path}`)); } - writeConfig(config, opts.config); + writeConfig(config, opts.config, { allowInvalidReplace }); if (tc) trackInstallCompleted(tc, { adapterType: server.deploymentMode, diff --git a/cli/src/config/schema.ts b/cli/src/config/schema.ts index 65ddeab7334..c82af46f379 100644 --- a/cli/src/config/schema.ts +++ b/cli/src/config/schema.ts @@ -1,5 +1,6 @@ export { paperclipConfigSchema, + findConfigNearMatchWarnings, configMetaSchema, llmConfigSchema, databaseBackupConfigSchema, @@ -28,3 +29,5 @@ export { type SecretsLocalEncryptedConfig, type ConfigMeta, } from "../../../packages/shared/src/config-schema.js"; + +export { mergeConfigValues } from "../../../packages/shared/src/config-persistence.js"; diff --git a/cli/src/config/store.ts b/cli/src/config/store.ts index 8dddc777064..b39a92258b6 100644 --- a/cli/src/config/store.ts +++ b/cli/src/config/store.ts @@ -1,6 +1,13 @@ import fs from "node:fs"; import path from "node:path"; -import { paperclipConfigSchema, type PaperclipConfig } from "./schema.js"; +import { randomUUID } from "node:crypto"; +import { isDeepStrictEqual } from "node:util"; +import { + findConfigNearMatchWarnings, + mergeConfigValues, + paperclipConfigSchema, + type PaperclipConfig, +} from "./schema.js"; import { resolveDefaultConfigPath, resolvePaperclipInstanceId, @@ -83,26 +90,122 @@ function formatValidationError(err: unknown): string { return err instanceof Error ? err.message : String(err); } -export function readConfig(configPath?: string): PaperclipConfig | null { - const filePath = resolveConfigPath(configPath); - if (!fs.existsSync(filePath)) return null; +function parseConfig(filePath: string, options: { warnNearMatches?: boolean } = {}): PaperclipConfig { const raw = parseJson(filePath); const migrated = migrateLegacyConfig(raw); const parsed = paperclipConfigSchema.safeParse(migrated); if (!parsed.success) { throw new Error(`Invalid config at ${filePath}: ${formatValidationError(parsed.error)}`); } + if (options.warnNearMatches !== false) { + for (const warning of findConfigNearMatchWarnings(raw)) { + console.warn(`Warning: ${warning}`); + } + } return parsed.data; } +function writeFileAtomic(filePath: string, contents: string): void { + const temporaryPath = path.join( + path.dirname(filePath), + `.${path.basename(filePath)}.tmp-${process.pid}-${randomUUID()}`, + ); + try { + fs.writeFileSync(temporaryPath, contents, { mode: 0o600, flag: "wx" }); + fs.renameSync(temporaryPath, filePath); + } finally { + fs.rmSync(temporaryPath, { force: true }); + } +} + +function effectiveConfigEquals(source: PaperclipConfig, candidate: PaperclipConfig): boolean { + const normalizedCandidate: PaperclipConfig = { + ...candidate, + $meta: { + ...candidate.$meta, + source: source.$meta.source, + updatedAt: source.$meta.updatedAt, + }, + }; + return isDeepStrictEqual(source, normalizedCandidate); +} + +export type ConfigReadState = + | { status: "missing"; path: string } + | { status: "valid"; path: string; config: PaperclipConfig } + | { status: "invalid"; path: string; error: Error }; + +export function readConfigState(configPath?: string): ConfigReadState { + const filePath = resolveConfigPath(configPath); + if (!fs.existsSync(filePath)) return { status: "missing", path: filePath }; + try { + return { status: "valid", path: filePath, config: parseConfig(filePath) }; + } catch (error) { + return { + status: "invalid", + path: filePath, + error: error instanceof Error ? error : new Error(String(error)), + }; + } +} + +export function backupInvalidConfig(configPath?: string): string { + const filePath = resolveConfigPath(configPath); + if (!fs.existsSync(filePath)) { + throw new Error(`Cannot back up missing config at ${filePath}`); + } + + for (let index = 1; ; index += 1) { + const backupPath = `${filePath}.invalid-${index}`; + try { + fs.copyFileSync(filePath, backupPath, fs.constants.COPYFILE_EXCL); + fs.chmodSync(backupPath, 0o600); + return backupPath; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") continue; + throw error; + } + } +} + +export function readConfig(configPath?: string): PaperclipConfig | null { + const filePath = resolveConfigPath(configPath); + if (!fs.existsSync(filePath)) return null; + return parseConfig(filePath); +} + export function writeConfig( config: PaperclipConfig, configPath?: string, -): void { + options: { allowInvalidReplace?: boolean } = {}, +): boolean { const filePath = resolveConfigPath(configPath); const dir = path.dirname(filePath); fs.mkdirSync(dir, { recursive: true }); + let sourceConfig: PaperclipConfig | null = null; + if (fs.existsSync(filePath)) { + try { + sourceConfig = parseConfig(filePath, { warnNearMatches: false }); + } catch (error) { + if (!options.allowInvalidReplace) { + throw new Error( + `Refusing to overwrite invalid config at ${filePath}. Back it up and explicitly confirm repair first. ${error instanceof Error ? error.message : String(error)}`, + ); + } + } + } + + const merged = sourceConfig ? mergeConfigValues(sourceConfig, config) : config; + const parsed = paperclipConfigSchema.safeParse(merged); + if (!parsed.success) { + throw new Error(`Invalid config update for ${filePath}: ${formatValidationError(parsed.error)}`); + } + + if (sourceConfig && effectiveConfigEquals(sourceConfig, parsed.data)) { + return false; + } + // Backup existing config before overwriting if (fs.existsSync(filePath)) { const backupPath = filePath + ".backup"; @@ -110,9 +213,8 @@ export function writeConfig( fs.chmodSync(backupPath, 0o600); } - fs.writeFileSync(filePath, JSON.stringify(config, null, 2) + "\n", { - mode: 0o600, - }); + writeFileAtomic(filePath, JSON.stringify(parsed.data, null, 2) + "\n"); + return true; } export function configExists(configPath?: string): boolean { diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 436c7bc85c2..27e5122b9a7 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -415,6 +415,10 @@ Local adapters require their corresponding CLI/session setup on the machine runn Agent, project, environment, secret, skill, and workspace config edits are sampled at the next run boundary. A heartbeat that is already running finishes with the config it started with. +Paperclip preserves unknown top-level and nested `config.json` keys during known-field updates so extensions and newer-version settings survive older CLI/server writers. Likely misspellings of managed keys produce a warning but remain untouched. Semantically unchanged updates skip the write so file bytes and mtimes stay stable. + +If `config.json` exists but cannot be parsed or validated, `paperclipai configure` and `paperclipai onboard` first create a byte-for-byte sibling backup using the next available deterministic name (`config.json.invalid-1`, `config.json.invalid-2`, and so on). Non-interactive runs then stop without replacing the original. Interactive repair requires explicit confirmation and stages the defaults before atomically replacing the invalid file; the command prints the preserved backup path before asking. + When effective run config changes, Paperclip may intentionally skip a saved adapter session, refresh persisted workspace runtime config, replace a reused execution workspace, or avoid reusing a sandbox/environment lease. Fresh execution can lose adapter-specific session, workspace, or sandbox state; correctness of the next run's config takes priority over continuity. Plain environment values affect freshness through value hashes; run result JSON and workspace operation logs expose only the non-sensitive freshness decision categories, without storing secret values, full env maps, provider credentials, or private path details. ## Worktree-local Instances diff --git a/packages/shared/src/config-persistence.ts b/packages/shared/src/config-persistence.ts new file mode 100644 index 00000000000..18c0b7cf4c0 --- /dev/null +++ b/packages/shared/src/config-persistence.ts @@ -0,0 +1,27 @@ +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * Applies an owned config update over the parsed source while retaining unknown + * extension keys. An explicit `undefined` removes an owned key; arrays and + * scalar values replace their source value. + */ +export function mergeConfigValues(source: T, update: T): T { + if (!isRecord(source) || !isRecord(update)) return update; + + const merged: Record = { ...source }; + for (const [key, updateValue] of Object.entries(update)) { + if (updateValue === undefined) { + delete merged[key]; + continue; + } + + const sourceValue = source[key]; + merged[key] = isRecord(sourceValue) && isRecord(updateValue) + ? mergeConfigValues(sourceValue, updateValue) + : updateValue; + } + + return merged as T; +} diff --git a/packages/shared/src/config-schema.test.ts b/packages/shared/src/config-schema.test.ts index 9e2a75ccf78..db77ce1ad8b 100644 --- a/packages/shared/src/config-schema.test.ts +++ b/packages/shared/src/config-schema.test.ts @@ -1,22 +1,26 @@ import { describe, expect, it } from "vitest"; -import { paperclipConfigSchema } from "./config-schema.js"; +import { findConfigNearMatchWarnings, paperclipConfigSchema } from "./config-schema.js"; + +function minimalConfig() { + return { + $meta: { + version: 1, + updatedAt: "2026-05-10T00:00:00.000Z", + source: "configure", + }, + database: { + mode: "embedded-postgres", + }, + logging: { + mode: "file", + }, + server: {}, + }; +} describe("paperclip config schema", () => { it("defaults omitted runtime paths to legacy instance-root locations", () => { - const parsed = paperclipConfigSchema.parse({ - $meta: { - version: 1, - updatedAt: "2026-05-10T00:00:00.000Z", - source: "configure", - }, - database: { - mode: "embedded-postgres", - }, - logging: { - mode: "file", - }, - server: {}, - }); + const parsed = paperclipConfigSchema.parse(minimalConfig()); expect(parsed.database.embeddedPostgresDataDir).toBe("~/.paperclip/instances/default/db"); expect(parsed.database.backup.dir).toBe("~/.paperclip/instances/default/data/backups"); @@ -24,4 +28,39 @@ describe("paperclip config schema", () => { expect(parsed.storage.localDisk.baseDir).toBe("~/.paperclip/instances/default/data/storage"); expect(parsed.secrets.localEncrypted.keyFilePath).toBe("~/.paperclip/instances/default/secrets/master.key"); }); + + it("preserves unknown top-level and nested provider keys", () => { + const parsed = paperclipConfigSchema.parse({ + ...minimalConfig(), + futurePlugin: { enabled: true }, + storage: { + provider: "local_disk", + localDisk: { + futureMountOption: "cached", + }, + }, + }); + + expect(parsed.futurePlugin).toEqual({ enabled: true }); + expect(parsed.storage.localDisk.futureMountOption).toBe("cached"); + }); + + it("still rejects invalid values for known fields", () => { + expect(() => paperclipConfigSchema.parse({ + ...minimalConfig(), + server: { port: "3100" }, + })).toThrow(); + }); + + it("warns about near-match keys without rejecting them", () => { + const raw = { + ...minimalConfig(), + server: { potr: 3200 }, + }; + + expect(paperclipConfigSchema.parse(raw).server.potr).toBe(3200); + expect(findConfigNearMatchWarnings(raw)).toEqual([ + "Unknown config key server.potr; did you mean server.port? The unknown key will be preserved.", + ]); + }); }); diff --git a/packages/shared/src/config-schema.ts b/packages/shared/src/config-schema.ts index 229abd1a02d..45fe5706ca4 100644 --- a/packages/shared/src/config-schema.ts +++ b/packages/shared/src/config-schema.ts @@ -13,19 +13,19 @@ export const configMetaSchema = z.object({ version: z.literal(1), updatedAt: z.string(), source: z.enum(["onboard", "configure", "doctor"]), -}); +}).passthrough(); export const llmConfigSchema = z.object({ provider: z.enum(["claude", "openai"]), apiKey: z.string().optional(), -}); +}).passthrough(); export const databaseBackupConfigSchema = z.object({ enabled: z.boolean().default(true), intervalMinutes: z.number().int().min(1).max(7 * 24 * 60).default(60), retentionDays: z.number().int().min(1).max(3650).default(7), dir: z.string().default("~/.paperclip/instances/default/data/backups"), -}); +}).passthrough(); export const databaseConfigSchema = z.object({ mode: z.enum(["embedded-postgres", "postgres"]).default("embedded-postgres"), @@ -38,12 +38,12 @@ export const databaseConfigSchema = z.object({ retentionDays: 7, dir: "~/.paperclip/instances/default/data/backups", }), -}); +}).passthrough(); export const loggingConfigSchema = z.object({ mode: z.enum(["file", "cloud"]), logDir: z.string().default("~/.paperclip/instances/default/logs"), -}); +}).passthrough(); export const serverConfigSchema = z.object({ deploymentMode: z.enum(DEPLOYMENT_MODES).default("local_trusted"), @@ -54,17 +54,17 @@ export const serverConfigSchema = z.object({ port: z.number().int().min(1).max(65535).default(3100), allowedHostnames: z.array(z.string().min(1)).default([]), serveUi: z.boolean().default(true), -}); +}).passthrough(); export const authConfigSchema = z.object({ baseUrlMode: z.enum(AUTH_BASE_URL_MODES).default("auto"), publicBaseUrl: z.string().url().optional(), disableSignUp: z.boolean().default(false), -}); +}).passthrough(); export const storageLocalDiskConfigSchema = z.object({ baseDir: z.string().default("~/.paperclip/instances/default/data/storage"), -}); +}).passthrough(); export const storageS3ConfigSchema = z.object({ bucket: z.string().min(1).default("paperclip"), @@ -72,7 +72,7 @@ export const storageS3ConfigSchema = z.object({ endpoint: z.string().optional(), prefix: z.string().default(""), forcePathStyle: z.boolean().default(false), -}); +}).passthrough(); export const storageConfigSchema = z.object({ provider: z.enum(STORAGE_PROVIDERS).default("local_disk"), @@ -85,11 +85,11 @@ export const storageConfigSchema = z.object({ prefix: "", forcePathStyle: false, }), -}); +}).passthrough(); export const secretsLocalEncryptedConfigSchema = z.object({ keyFilePath: z.string().default("~/.paperclip/instances/default/secrets/master.key"), -}); +}).passthrough(); export const secretsConfigSchema = z.object({ provider: z.enum(SECRET_PROVIDERS).default("local_encrypted"), @@ -97,15 +97,15 @@ export const secretsConfigSchema = z.object({ localEncrypted: secretsLocalEncryptedConfigSchema.default({ keyFilePath: "~/.paperclip/instances/default/secrets/master.key", }), -}); +}).passthrough(); export const telemetryConfigSchema = z.object({ enabled: z.boolean().default(true), -}).default({}); +}).passthrough().default({}); export const updatesConfigSchema = z.object({ checkEnabled: z.boolean().default(true), -}).default({}); +}).passthrough().default({}); export const paperclipConfigSchema = z .object({ @@ -140,6 +140,7 @@ export const paperclipConfigSchema = z }, }), }) + .passthrough() .superRefine((value, ctx) => { if (value.server.deploymentMode === "local_trusted" && value.server.exposure !== "private") { ctx.addIssue({ @@ -188,6 +189,94 @@ export const paperclipConfigSchema = z } }); +const CONFIG_OBJECT_KEYS = new Map([ + ["", ["$meta", "llm", "database", "logging", "server", "telemetry", "updates", "auth", "storage", "secrets"]], + ["$meta", ["version", "updatedAt", "source"]], + ["llm", ["provider", "apiKey"]], + ["database", ["mode", "connectionString", "embeddedPostgresDataDir", "embeddedPostgresPort", "backup"]], + ["database.backup", ["enabled", "intervalMinutes", "retentionDays", "dir"]], + ["logging", ["mode", "logDir"]], + ["server", ["deploymentMode", "exposure", "bind", "customBindHost", "host", "port", "allowedHostnames", "serveUi"]], + ["telemetry", ["enabled"]], + ["updates", ["checkEnabled"]], + ["auth", ["baseUrlMode", "publicBaseUrl", "disableSignUp"]], + ["storage", ["provider", "localDisk", "s3"]], + ["storage.localDisk", ["baseDir"]], + ["storage.s3", ["bucket", "region", "endpoint", "prefix", "forcePathStyle"]], + ["secrets", ["provider", "strictMode", "localEncrypted"]], + ["secrets.localEncrypted", ["keyFilePath"]], +]); + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function valueAtPath(value: unknown, objectPath: string): Record | null { + let current = value; + for (const segment of objectPath.split(".").filter(Boolean)) { + if (!isRecord(current)) return null; + current = current[segment]; + } + return isRecord(current) ? current : null; +} + +function editDistance(left: string, right: string): number { + if (left.length === right.length) { + const differences = Array.from(left, (character, index) => character === right[index] ? -1 : index) + .filter((index) => index >= 0); + if ( + differences.length === 2 + && differences[1] === differences[0]! + 1 + && left[differences[0]!] === right[differences[1]!] + && left[differences[1]!] === right[differences[0]!] + ) { + return 1; + } + } + + const previous = Array.from({ length: right.length + 1 }, (_, index) => index); + for (let leftIndex = 1; leftIndex <= left.length; leftIndex += 1) { + const current = [leftIndex]; + for (let rightIndex = 1; rightIndex <= right.length; rightIndex += 1) { + current[rightIndex] = Math.min( + (current[rightIndex - 1] ?? 0) + 1, + (previous[rightIndex] ?? 0) + 1, + (previous[rightIndex - 1] ?? 0) + (left[leftIndex - 1] === right[rightIndex - 1] ? 0 : 1), + ); + } + previous.splice(0, previous.length, ...current); + } + return previous[right.length] ?? right.length; +} + +/** + * Reports likely misspellings without rejecting or deleting extension keys. + * Unknown keys that are not close to a managed key remain silent passthrough data. + */ +export function findConfigNearMatchWarnings(raw: unknown): string[] { + const warnings: string[] = []; + + for (const [objectPath, knownKeys] of CONFIG_OBJECT_KEYS) { + const object = valueAtPath(raw, objectPath); + if (!object) continue; + + for (const key of Object.keys(object)) { + if (knownKeys.includes(key)) continue; + const nearMatch = knownKeys + .map((knownKey) => ({ knownKey, distance: editDistance(key.toLowerCase(), knownKey.toLowerCase()) })) + .filter(({ knownKey, distance }) => distance <= (knownKey.length >= 8 ? 2 : 1)) + .sort((left, right) => left.distance - right.distance || left.knownKey.localeCompare(right.knownKey))[0]; + if (!nearMatch) continue; + + const keyPath = objectPath ? `${objectPath}.${key}` : key; + const matchPath = objectPath ? `${objectPath}.${nearMatch.knownKey}` : nearMatch.knownKey; + warnings.push(`Unknown config key ${keyPath}; did you mean ${matchPath}? The unknown key will be preserved.`); + } + } + + return warnings; +} + export type PaperclipConfig = z.infer; export type LlmConfig = z.infer; export type DatabaseConfig = z.infer; diff --git a/server/src/__tests__/config-file.test.ts b/server/src/__tests__/config-file.test.ts index 9d5b2701ca7..76552681b33 100644 --- a/server/src/__tests__/config-file.test.ts +++ b/server/src/__tests__/config-file.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { readConfigFile } from "../config-file.js"; const ORIGINAL_PAPERCLIP_CONFIG = process.env.PAPERCLIP_CONFIG; @@ -49,6 +49,7 @@ describe("readConfigFile", () => { } fs.rmSync(tempDir, { recursive: true, force: true }); + vi.restoreAllMocks(); }); it("returns null when the config file does not exist", () => { @@ -89,4 +90,18 @@ describe("readConfigFile", () => { }, }); }); + + it("preserves near-match keys and warns instead of deleting them", () => { + const config = minimalConfig() as { server: Record }; + config.server.potr = 3200; + writeConfig(configPath, config); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const parsed = readConfigFile(); + + expect(parsed?.server.potr).toBe(3200); + expect(warn).toHaveBeenCalledWith( + "Warning: Unknown config key server.potr; did you mean server.port? The unknown key will be preserved.", + ); + }); }); diff --git a/server/src/__tests__/worktree-config.test.ts b/server/src/__tests__/worktree-config.test.ts index 1ea6859f99e..706d84882ec 100644 --- a/server/src/__tests__/worktree-config.test.ts +++ b/server/src/__tests__/worktree-config.test.ts @@ -331,6 +331,53 @@ describe("worktree config repair", () => { expect(writtenConfig.database.embeddedPostgresPort).toBe(54329); }); + it("preserves unknown config keys and skips unchanged writes through the server writer", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-passthrough-")); + const worktreeRoot = path.join(tempRoot, "PAP-16582-config-passthrough"); + const paperclipDir = path.join(worktreeRoot, ".paperclip"); + const configPath = path.join(paperclipDir, "config.json"); + const isolatedHome = path.join(tempRoot, ".paperclip-worktrees"); + const instanceRoot = path.join(isolatedHome, "instances", "pap-16582-config-passthrough"); + const config = { + ...buildIsolatedConfig(instanceRoot, 3101, 54330), + futurePlugin: { enabled: true }, + server: { + ...buildIsolatedConfig(instanceRoot, 3101, 54330).server, + futureProxyMode: "tunnel", + }, + }; + + await fs.mkdir(paperclipDir, { recursive: true }); + await fs.writeFile(configPath, `${JSON.stringify(config, null, 2)}\n`, "utf8"); + await fs.writeFile( + path.join(paperclipDir, ".env"), + ["PAPERCLIP_IN_WORKTREE=true", "PAPERCLIP_WORKTREE_NAME=PAP-16582-config-passthrough", ""].join("\n"), + "utf8", + ); + + process.chdir(worktreeRoot); + process.env.PAPERCLIP_IN_WORKTREE = "true"; + process.env.PAPERCLIP_WORKTREE_NAME = "PAP-16582-config-passthrough"; + process.env.PAPERCLIP_HOME = isolatedHome; + process.env.PAPERCLIP_INSTANCE_ID = "pap-16582-config-passthrough"; + process.env.PAPERCLIP_CONFIG = configPath; + delete process.env.PORT; + delete process.env.DATABASE_URL; + + maybePersistWorktreeRuntimePorts({ serverPort: 3200, databasePort: 54400 }); + + const writtenConfig = JSON.parse(await fs.readFile(configPath, "utf8")); + expect(writtenConfig.server.port).toBe(3200); + expect(writtenConfig.database.embeddedPostgresPort).toBe(54400); + expect(writtenConfig.futurePlugin).toEqual({ enabled: true }); + expect(writtenConfig.server.futureProxyMode).toBe("tunnel"); + + const stableTime = new Date("2026-01-01T00:00:00.000Z"); + await fs.utimes(configPath, stableTime, stableTime); + maybePersistWorktreeRuntimePorts({ serverPort: 3200, databasePort: 54400 }); + expect((await fs.stat(configPath)).mtimeMs).toBe(stableTime.getTime()); + }); + it("does not adopt a .paperclip config whose own env does not declare a worktree", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-unattested-")); const repoRoot = path.join(tempRoot, "repo"); diff --git a/server/src/config-file.ts b/server/src/config-file.ts index 9c150f4a782..30c9ca9f412 100644 --- a/server/src/config-file.ts +++ b/server/src/config-file.ts @@ -1,5 +1,9 @@ import fs from "node:fs"; -import { paperclipConfigSchema, type PaperclipConfig } from "@paperclipai/shared"; +import { + paperclipConfigSchema, + type PaperclipConfig, +} from "@paperclipai/shared"; +import { findConfigNearMatchWarnings } from "@paperclipai/shared/config-schema"; import { ZodError } from "zod"; import { resolvePaperclipConfigPath } from "./paths.js"; @@ -26,7 +30,11 @@ export function readConfigFile(): PaperclipConfig | null { } try { - return paperclipConfigSchema.parse(raw); + const parsed = paperclipConfigSchema.parse(raw); + for (const warning of findConfigNearMatchWarnings(raw)) { + console.warn(`Warning: ${warning}`); + } + return parsed; } catch (error) { if (error instanceof ZodError) { throw new Error(`Invalid Paperclip config at ${configPath}: ${formatConfigValidationError(error)}`); diff --git a/server/src/worktree-config.ts b/server/src/worktree-config.ts index de8d824c18c..aff9bde56b1 100644 --- a/server/src/worktree-config.ts +++ b/server/src/worktree-config.ts @@ -1,7 +1,13 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import type { PaperclipConfig } from "@paperclipai/shared"; +import { randomUUID } from "node:crypto"; +import { isDeepStrictEqual } from "node:util"; +import { + paperclipConfigSchema, + type PaperclipConfig, +} from "@paperclipai/shared"; +import { mergeConfigValues } from "@paperclipai/shared/config-persistence"; import { resolvePaperclipConfigPath, resolvePaperclipEnvPath } from "./paths.js"; function nonEmpty(value: string | null | undefined): string | null { @@ -305,9 +311,32 @@ function resolveWorktreeRuntimeContext( }; } -function writeConfigFile(configPath: string, config: PaperclipConfig): void { +function writeConfigFile(configPath: string, config: PaperclipConfig): boolean { fs.mkdirSync(path.dirname(configPath), { recursive: true }); - fs.writeFileSync(configPath, JSON.stringify(config, null, 2) + "\n", { mode: 0o600 }); + let sourceConfig: PaperclipConfig | null = null; + if (fs.existsSync(configPath)) { + const raw = JSON.parse(fs.readFileSync(configPath, "utf8")) as unknown; + sourceConfig = paperclipConfigSchema.parse(raw); + } + + const merged = sourceConfig ? mergeConfigValues(sourceConfig, config) : config; + const parsed = paperclipConfigSchema.parse(merged); + if (sourceConfig && isDeepStrictEqual(sourceConfig, parsed)) return false; + + const temporaryPath = path.join( + path.dirname(configPath), + `.${path.basename(configPath)}.tmp-${process.pid}-${randomUUID()}`, + ); + try { + fs.writeFileSync(temporaryPath, JSON.stringify(parsed, null, 2) + "\n", { + mode: 0o600, + flag: "wx", + }); + fs.renameSync(temporaryPath, configPath); + } finally { + fs.rmSync(temporaryPath, { force: true }); + } + return true; } function resolveRepoManagedWorktreesRoot(worktreeRoot: string): string | null { @@ -585,8 +614,7 @@ export function maybeRepairLegacyWorktreeConfigAndEnvFiles(): { serverPort: selectedServerPort, databasePort: selectedDatabasePort, }); - writeConfigFile(context.configPath, selectedConfig); - repairedConfig = true; + repairedConfig = writeConfigFile(context.configPath, selectedConfig); if (serverPortCollision || databasePortCollision) { console.warn(