Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/lib/config/services/project.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
40 changes: 37 additions & 3 deletions src/lib/config/services/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,46 @@ export function loadProjectJsonConfig<ConfigType = Config>(
fs.readFileSync(resolvedPath, 'utf-8')
) as Partial<Config> & { $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) {
Expand Down
Loading