diff --git a/.changeset/config-schema-directed-set.md b/.changeset/config-schema-directed-set.md new file mode 100644 index 0000000..cb26345 --- /dev/null +++ b/.changeset/config-schema-directed-set.md @@ -0,0 +1,12 @@ +--- +'@imdeadpool/colony': patch +--- + +`colony config set` now coerces values using the settings schema instead +of a regex. The old heuristic parsed anything matching `^-?\d+$` as a +number — so `colony config set embedding.model 1.0` silently stored the +number `1`. The new logic walks `SettingsSchema` to the target field and +coerces only when the leaf type calls for it (booleans → bool, numbers +→ number, arrays/objects/records → JSON, enums and strings → raw). Zod +still validates the final result, so malformed input is rejected with a +shape-aware error rather than coerced into the wrong JS type. diff --git a/apps/cli/src/commands/config.ts b/apps/cli/src/commands/config.ts index 05b4c2f..2ecc741 100644 --- a/apps/cli/src/commands/config.ts +++ b/apps/cli/src/commands/config.ts @@ -37,20 +37,95 @@ function setDotted(obj: Record, path: string, value: unknown): if (last) cur[last] = value; } -function coerce(raw: string): unknown { - if (raw === 'true') return true; - if (raw === 'false') return false; - if (raw === 'null') return null; - if (/^-?\d+$/.test(raw)) return Number(raw); - if (/^-?\d*\.\d+$/.test(raw)) return Number(raw); - // Try JSON for arrays/objects; fall back to raw string. - if (raw.startsWith('[') || raw.startsWith('{')) { +// Walk the zod schema so `colony config set` coerces values with the shape of +// the target field instead of a regex heuristic. Compare against `_def.typeName` +// strings instead of `instanceof z.ZodFoo`: the CLI bundles its own zod copy +// via tsup, so schemas returned from `@colony/config` can originate in a +// different realpath and fail nominal `instanceof` checks even though the +// runtime shape is identical. +type ZodTypeName = + | 'ZodDefault' + | 'ZodOptional' + | 'ZodNullable' + | 'ZodObject' + | 'ZodRecord' + | 'ZodBoolean' + | 'ZodNumber' + | 'ZodString' + | 'ZodArray' + | 'ZodEnum'; + +type ZodNode = { + _def: { typeName: ZodTypeName } & Record; + shape?: Record; +}; + +function typeName(schema: unknown): ZodTypeName | undefined { + const def = (schema as ZodNode | undefined)?._def; + const n = def?.typeName; + return typeof n === 'string' ? (n as ZodTypeName) : undefined; +} + +function unwrap(schema: ZodNode): ZodNode { + let cur = schema; + let n = typeName(cur); + while (n === 'ZodDefault' || n === 'ZodOptional' || n === 'ZodNullable') { + cur = cur._def.innerType as ZodNode; + n = typeName(cur); + } + return cur; +} + +export function leafSchema(root: unknown, path: string): unknown { + let cur = unwrap(root as ZodNode); + for (const part of path.split('.')) { + const t = typeName(cur); + if (t === 'ZodObject') { + const shape = (cur as { shape: Record }).shape; + const child = shape[part]; + if (!child) return undefined; + cur = unwrap(child); + continue; + } + if (t === 'ZodRecord') { + cur = unwrap(cur._def.valueType as ZodNode); + continue; + } + return undefined; + } + return cur; +} + +export function coerceForPath(raw: string, path: string): unknown { + const leaf = leafSchema(SettingsSchema, path); + if (!leaf) { + // Unknown path — hand the raw string to validation so the zod error is the + // source of truth instead of a separate "unknown key" message that would hide + // the schema shape from the user. + return raw; + } + const t = typeName(leaf); + if (t === 'ZodBoolean') { + if (raw === 'true') return true; + if (raw === 'false') return false; + return raw; + } + if (t === 'ZodNumber') { + if (raw.trim() === '') return raw; + const n = Number(raw); + return Number.isFinite(n) ? n : raw; + } + if (t === 'ZodArray' || t === 'ZodObject' || t === 'ZodRecord') { try { return JSON.parse(raw); } catch { - // fall through + return raw; } } + if (t === 'ZodEnum' || t === 'ZodString') { + return raw; + } + if (raw === 'null') return null; return raw; } @@ -114,7 +189,8 @@ export function registerConfigCommand(program: Command): void { .action((key: string, value: string) => { const settings = loadSettings(); const next = JSON.parse(JSON.stringify(settings)) as Record; - setDotted(next, key, coerce(value)); + const coerced = coerceForPath(value, key); + setDotted(next, key, coerced); const parsed = SettingsSchema.safeParse(next); if (!parsed.success) { process.stderr.write(`${kleur.red('invalid:')} ${parsed.error.message}\n`); @@ -122,7 +198,7 @@ export function registerConfigCommand(program: Command): void { return; } saveSettings(parsed.data); - process.stdout.write(`${kleur.green('✓')} ${key} = ${JSON.stringify(coerce(value))}\n`); + process.stdout.write(`${kleur.green('✓')} ${key} = ${JSON.stringify(coerced)}\n`); }); cfg diff --git a/apps/cli/test/config.test.ts b/apps/cli/test/config.test.ts new file mode 100644 index 0000000..7d277c8 --- /dev/null +++ b/apps/cli/test/config.test.ts @@ -0,0 +1,53 @@ +import { SettingsSchema } from '@colony/config'; +import { describe, expect, it } from 'vitest'; +import { coerceForPath, leafSchema } from '../src/commands/config.js'; + +describe('coerceForPath (schema-directed)', () => { + it('parses numeric settings as numbers even when the string looks like a version', () => { + expect(coerceForPath('1', 'workerPort')).toBe(1); + expect(coerceForPath('37777', 'workerPort')).toBe(37777); + expect(coerceForPath('0.25', 'search.alpha')).toBe(0.25); + }); + + it('keeps string settings as strings instead of coercing number-looking ones', () => { + expect(coerceForPath('1.0', 'embedding.model')).toBe('1.0'); + expect(coerceForPath('0.5', 'embedding.model')).toBe('0.5'); + expect(coerceForPath('Xenova/all-MiniLM-L6-v2', 'embedding.model')).toBe( + 'Xenova/all-MiniLM-L6-v2', + ); + }); + + it('parses booleans only for boolean fields', () => { + expect(coerceForPath('true', 'embedding.autoStart')).toBe(true); + expect(coerceForPath('false', 'privacy.redactSecrets')).toBe(false); + // A string field named after "true" stays a string + expect(coerceForPath('true', 'embedding.model')).toBe('true'); + }); + + it('parses arrays and records via JSON', () => { + expect(coerceForPath('["node_modules/**"]', 'privacy.excludePatterns')).toEqual([ + 'node_modules/**', + ]); + expect(coerceForPath('{"cursor":true}', 'ides')).toEqual({ cursor: true }); + }); + + it('leaves enum values as raw strings so zod can reject unknown members', () => { + expect(coerceForPath('ultra', 'compression.intensity')).toBe('ultra'); + expect(coerceForPath('garbage', 'compression.intensity')).toBe('garbage'); + }); + + it('hands unknown paths back as raw strings so schema validation produces the error', () => { + expect(coerceForPath('anything', 'does.not.exist')).toBe('anything'); + }); +}); + +describe('leafSchema', () => { + it('walks nested object paths', () => { + expect(leafSchema(SettingsSchema, 'embedding.provider')).toBeDefined(); + expect(leafSchema(SettingsSchema, 'search.alpha')).toBeDefined(); + }); + + it('returns undefined for unknown paths', () => { + expect(leafSchema(SettingsSchema, 'bogus.path')).toBeUndefined(); + }); +});