diff --git a/src/lib/config/services/project.test.ts b/src/lib/config/services/project.test.ts index 5618d899..cef14531 100644 --- a/src/lib/config/services/project.test.ts +++ b/src/lib/config/services/project.test.ts @@ -56,4 +56,43 @@ describe('loadProjectConfig', () => { expect(config.service.endpoint).toBe('http://localhost:11434') } }) + + it('warns instead of throwing when the merged config fails validation', () => { + // Reproduces the user-reported crash: the merged config has fields + // the schema rejects (often inherited from a stale XDG / git + // config layer), and the previous behavior threw, taking down + // every coco command. The fix surfaces a warning so the user + // can fix the offending field but keeps the merge so the rest + // of the tool stays usable. + mockFs.existsSync.mockReturnValue(true) + // Project file has a valid local override (conventionalCommits) + // but the merged result will inherit an invalid service shape + // from the incoming `config` argument. + mockFs.readFileSync.mockReturnValue( + JSON.stringify({ conventionalCommits: true }) + ) + + const invalidIncoming = { + service: { + // Missing required fields + extra unknown field — guaranteed + // to fail any anyOf branch in the service schema. + provider: 'openai', + model: 'gpt-4o', + unknownDriftField: 'something', + }, + } as unknown as Config + + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}) + + expect(() => loadProjectJsonConfig(invalidIncoming)).not.toThrow() + + const result = loadProjectJsonConfig(invalidIncoming) + // Local override still merged in despite validation failure — + // the tool keeps working with the user's intent applied. + expect((result as unknown as { conventionalCommits?: boolean }).conventionalCommits).toBe(true) + expect(warnSpy).toHaveBeenCalled() + expect(warnSpy.mock.calls[0][0]).toContain('config validation issues detected') + + warnSpy.mockRestore() + }) }) diff --git a/src/lib/config/services/project.ts b/src/lib/config/services/project.ts index 6377a30c..0566a7c2 100644 --- a/src/lib/config/services/project.ts +++ b/src/lib/config/services/project.ts @@ -45,12 +45,46 @@ export function loadProjectJsonConfig( fs.readFileSync(resolvedPath, 'utf-8') ) as Partial & { $schema: string } - config = { ...config, ...projectConfig } as Config + const merged = { ...config, ...projectConfig } as Config - const isProjectConfigValid = validate(config) + // Validate the merged result, but DON'T throw on failure. + // Reasons: + // + // 1. Throwing turns "you have a stale field somewhere in your + // config" into "the whole tool fails to boot" — every coco + // command that loads config crashes. That's a brutal + // regression cost for what's almost always a recoverable + // situation. + // + // 2. The validation runs on the MERGED config, so the actual + // offending file might be the XDG / git / env layer, not + // the local project file we're nominally loading. Pinning + // the error to "project config" was actively misleading. + // + // 3. We have sane defaults. A drifted service shape can fall + // back to the default service and the rest of the tool + // keeps working — the user still gets their commits, their + // changelogs, their PR flows, etc. + // + // Instead: warn loudly to stderr (with the file path AND the + // specific schema errors) so the user knows exactly what to fix, + // and apply the merge so the user's intent still flows through. + // If a downstream component genuinely can't operate without a + // valid service shape, it can guard against that locally with a + // clear error rather than blowing up at config-load time. + const isProjectConfigValid = validate(merged) if (!isProjectConfigValid) { - throw new Error('Invalid project config', { cause: ajv.errorsText(validate.errors) }) + const errors = ajv.errorsText(validate.errors) + console.warn( + `[coco] Warning: config validation issues detected (continuing with merged config).\n` + + ` Local file: ${resolvedPath}\n` + + ` Schema issues: ${errors}\n` + + ` Fix the offending fields to silence this warning. The validation runs ` + + `against the merged result of every config source (defaults, XDG, git, project, env) ` + + `— the issue may be in any of those, not necessarily this file.` + ) } + config = merged } if (opts?.returnSource) {