From 06f4bdc7f49e8f2564a61dd4685c3a1390259c20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=82CHES?= Date: Sun, 15 Mar 2026 17:09:58 -0600 Subject: [PATCH] feat(preferences): add schema validation with unknown key detection (#542) Validates parsed preferences against known keys and expected types. Unknown keys produce warnings instead of being silently ignored. Previously unvalidated fields (budget_enforcement, context_pause_threshold, models, auto_supervisor, notifications, remote_questions) are now type-checked. Warnings surface through LoadedGSDPreferences so callers can inspect validation results. Closes #522 Co-authored-by: Claude Opus 4.6 (1M context) --- src/resources/extensions/gsd/preferences.ts | 107 ++++++++++- .../preferences-schema-validation.test.ts | 169 ++++++++++++++++++ 2 files changed, 275 insertions(+), 1 deletion(-) create mode 100644 src/resources/extensions/gsd/tests/preferences-schema-validation.test.ts diff --git a/src/resources/extensions/gsd/preferences.ts b/src/resources/extensions/gsd/preferences.ts index f44078da0..4c10936a5 100644 --- a/src/resources/extensions/gsd/preferences.ts +++ b/src/resources/extensions/gsd/preferences.ts @@ -15,6 +15,29 @@ const GLOBAL_PREFERENCES_PATH_UPPERCASE = join(homedir(), ".gsd", "PREFERENCES.m const PROJECT_PREFERENCES_PATH_UPPERCASE = join(process.cwd(), ".gsd", "PREFERENCES.md"); const SKILL_ACTIONS = new Set(["use", "prefer", "avoid"]); +/** All recognized top-level keys in GSDPreferences. Used to detect typos / stale config. */ +const KNOWN_PREFERENCE_KEYS = new Set([ + "version", + "always_use_skills", + "prefer_skills", + "avoid_skills", + "skill_rules", + "custom_instructions", + "models", + "skill_discovery", + "auto_supervisor", + "uat_dispatch", + "unique_milestone_ids", + "budget_ceiling", + "budget_enforcement", + "context_pause_threshold", + "notifications", + "remote_questions", + "git", + "post_unit_hooks", + "pre_dispatch_hooks", +]); + export interface GSDSkillRule { when: string; use?: string[]; @@ -105,6 +128,8 @@ export interface LoadedGSDPreferences { path: string; scope: "global" | "project"; preferences: GSDPreferences; + /** Validation warnings (unknown keys, type mismatches, deprecations). Empty when preferences are clean. */ + warnings?: string[]; } export function getGlobalGSDPreferencesPath(): string { @@ -138,10 +163,16 @@ export function loadEffectiveGSDPreferences(): LoadedGSDPreferences | null { if (!globalPreferences) return projectPreferences; if (!projectPreferences) return globalPreferences; + const mergedWarnings = [ + ...(globalPreferences.warnings ?? []), + ...(projectPreferences.warnings ?? []), + ]; + return { path: projectPreferences.path, scope: "project", preferences: mergePreferences(globalPreferences.preferences, projectPreferences.preferences), + ...(mergedWarnings.length > 0 ? { warnings: mergedWarnings } : {}), }; } @@ -367,10 +398,14 @@ function loadPreferencesFile(path: string, scope: "global" | "project"): LoadedG const preferences = parsePreferencesMarkdown(raw); if (!preferences) return null; + const validation = validatePreferences(preferences); + const allWarnings = [...validation.warnings, ...validation.errors]; + return { path, scope, - preferences, + preferences: validation.preferences, + ...(allWarnings.length > 0 ? { warnings: allWarnings } : {}), }; } @@ -633,6 +668,11 @@ function mergePreferences(base: GSDPreferences, override: GSDPreferences): GSDPr uat_dispatch: override.uat_dispatch ?? base.uat_dispatch, unique_milestone_ids: override.unique_milestone_ids ?? base.unique_milestone_ids, budget_ceiling: override.budget_ceiling ?? base.budget_ceiling, + budget_enforcement: override.budget_enforcement ?? base.budget_enforcement, + context_pause_threshold: override.context_pause_threshold ?? base.context_pause_threshold, + notifications: (base.notifications || override.notifications) + ? { ...(base.notifications ?? {}), ...(override.notifications ?? {}) } + : undefined, remote_questions: override.remote_questions ? { ...(base.remote_questions ?? {}), ...override.remote_questions } : base.remote_questions, @@ -653,6 +693,13 @@ export function validatePreferences(preferences: GSDPreferences): { const warnings: string[] = []; const validated: GSDPreferences = {}; + // ─── Unknown Key Detection ────────────────────────────────────────── + for (const key of Object.keys(preferences)) { + if (!KNOWN_PREFERENCE_KEYS.has(key)) { + warnings.push(`unknown preference key "${key}" — ignored`); + } + } + if (preferences.version !== undefined) { if (preferences.version === 1) { validated.version = 1; @@ -730,6 +777,64 @@ export function validatePreferences(preferences: GSDPreferences): { } } + // ─── Budget Enforcement ────────────────────────────────────────────── + if (preferences.budget_enforcement !== undefined) { + const validModes = new Set(["warn", "pause", "halt"]); + if (typeof preferences.budget_enforcement === "string" && validModes.has(preferences.budget_enforcement)) { + validated.budget_enforcement = preferences.budget_enforcement; + } else { + errors.push(`budget_enforcement must be one of: warn, pause, halt`); + } + } + + // ─── Context Pause Threshold ──────────────────────────────────────── + if (preferences.context_pause_threshold !== undefined) { + const raw = preferences.context_pause_threshold; + if (typeof raw === "number" && Number.isFinite(raw)) { + validated.context_pause_threshold = raw; + } else if (typeof raw === "string" && Number.isFinite(Number(raw))) { + validated.context_pause_threshold = Number(raw); + } else { + errors.push("context_pause_threshold must be a finite number"); + } + } + + // ─── Models ───────────────────────────────────────────────────────── + if (preferences.models !== undefined) { + if (preferences.models && typeof preferences.models === "object") { + validated.models = preferences.models; + } else { + errors.push("models must be an object"); + } + } + + // ─── Auto Supervisor ──────────────────────────────────────────────── + if (preferences.auto_supervisor !== undefined) { + if (preferences.auto_supervisor && typeof preferences.auto_supervisor === "object") { + validated.auto_supervisor = preferences.auto_supervisor; + } else { + errors.push("auto_supervisor must be an object"); + } + } + + // ─── Notifications ────────────────────────────────────────────────── + if (preferences.notifications !== undefined) { + if (preferences.notifications && typeof preferences.notifications === "object") { + validated.notifications = preferences.notifications; + } else { + errors.push("notifications must be an object"); + } + } + + // ─── Remote Questions ─────────────────────────────────────────────── + if (preferences.remote_questions !== undefined) { + if (preferences.remote_questions && typeof preferences.remote_questions === "object") { + validated.remote_questions = preferences.remote_questions; + } else { + errors.push("remote_questions must be an object"); + } + } + // ─── Post-Unit Hooks ───────────────────────────────────────────────── if (preferences.post_unit_hooks && Array.isArray(preferences.post_unit_hooks)) { const validHooks: PostUnitHookConfig[] = []; diff --git a/src/resources/extensions/gsd/tests/preferences-schema-validation.test.ts b/src/resources/extensions/gsd/tests/preferences-schema-validation.test.ts new file mode 100644 index 000000000..c79e82282 --- /dev/null +++ b/src/resources/extensions/gsd/tests/preferences-schema-validation.test.ts @@ -0,0 +1,169 @@ +/** + * preferences-schema-validation.test.ts — Validates that schema validation + * detects unknown keys, invalid types, and surfaces warnings correctly. + */ + +import { createTestContext } from "./test-helpers.ts"; +import { validatePreferences } from "../preferences.ts"; +import type { GSDPreferences } from "../preferences.ts"; + +const { assertEq, assertTrue, report } = createTestContext(); + +async function main(): Promise { + console.log("\n=== unknown keys produce warnings ==="); + + { + const prefs = { typo_key: "value" } as unknown as GSDPreferences; + const { warnings } = validatePreferences(prefs); + assertTrue(warnings.some(w => w.includes("typo_key")), "unknown key 'typo_key' produces warning"); + assertTrue(warnings.some(w => w.includes("unknown")), "warning mentions 'unknown'"); + } + + { + const prefs = { foo: 1, bar: 2 } as unknown as GSDPreferences; + const { warnings } = validatePreferences(prefs); + assertTrue(warnings.some(w => w.includes("foo")), "unknown key 'foo' produces warning"); + assertTrue(warnings.some(w => w.includes("bar")), "unknown key 'bar' produces warning"); + assertEq(warnings.filter(w => w.includes("unknown")).length, 2, "two unknown key warnings"); + } + + console.log("\n=== known keys do NOT produce unknown-key warnings ==="); + + { + const prefs: GSDPreferences = { + version: 1, + uat_dispatch: true, + budget_ceiling: 50, + skill_discovery: "auto", + }; + const { warnings } = validatePreferences(prefs); + const unknownWarnings = warnings.filter(w => w.includes("unknown")); + assertEq(unknownWarnings.length, 0, "valid keys produce no unknown-key warnings"); + } + + console.log("\n=== all GSDPreferences keys are accepted ==="); + + { + const prefs: GSDPreferences = { + version: 1, + always_use_skills: ["skill-a"], + prefer_skills: ["skill-b"], + avoid_skills: ["skill-c"], + skill_rules: [{ when: "testing", use: ["skill-d"] }], + custom_instructions: ["do a thing"], + models: { research: "claude-opus-4-6" }, + skill_discovery: "suggest", + auto_supervisor: { model: "claude-opus-4-6" }, + uat_dispatch: false, + unique_milestone_ids: true, + budget_ceiling: 100, + budget_enforcement: "warn", + context_pause_threshold: 0.8, + notifications: { enabled: true }, + remote_questions: { channel: "slack", channel_id: "C123" }, + git: { auto_push: true }, + post_unit_hooks: [{ name: "test-hook", after: ["execute-task"], prompt: "do it" }], + pre_dispatch_hooks: [{ name: "pre-hook", before: ["execute-task"], action: "skip" }], + }; + const { warnings } = validatePreferences(prefs); + const unknownWarnings = warnings.filter(w => w.includes("unknown")); + assertEq(unknownWarnings.length, 0, "all known keys produce no unknown-key warnings"); + } + + console.log("\n=== invalid value types produce errors ==="); + + { + const prefs = { budget_ceiling: "not-a-number" } as unknown as GSDPreferences; + const { errors, preferences } = validatePreferences(prefs); + assertTrue(errors.some(e => e.includes("budget_ceiling")), "invalid budget_ceiling produces error"); + assertEq(preferences.budget_ceiling, undefined, "invalid budget_ceiling falls back to undefined"); + } + + { + const prefs = { budget_enforcement: "invalid" } as unknown as GSDPreferences; + const { errors, preferences } = validatePreferences(prefs); + assertTrue(errors.some(e => e.includes("budget_enforcement")), "invalid budget_enforcement produces error"); + assertEq(preferences.budget_enforcement, undefined, "invalid budget_enforcement falls back to undefined"); + } + + { + const prefs = { context_pause_threshold: "not-a-number" } as unknown as GSDPreferences; + const { errors, preferences } = validatePreferences(prefs); + assertTrue(errors.some(e => e.includes("context_pause_threshold")), "invalid context_pause_threshold produces error"); + assertEq(preferences.context_pause_threshold, undefined, "invalid context_pause_threshold falls back to undefined"); + } + + { + const prefs = { skill_discovery: "invalid-mode" } as unknown as GSDPreferences; + const { errors, preferences } = validatePreferences(prefs); + assertTrue(errors.some(e => e.includes("skill_discovery")), "invalid skill_discovery produces error"); + assertEq(preferences.skill_discovery, undefined, "invalid skill_discovery falls back to undefined"); + } + + console.log("\n=== valid values pass through correctly ==="); + + { + const { preferences } = validatePreferences({ budget_enforcement: "halt" }); + assertEq(preferences.budget_enforcement, "halt", "valid budget_enforcement passes through"); + } + + { + const { preferences } = validatePreferences({ context_pause_threshold: 0.75 }); + assertEq(preferences.context_pause_threshold, 0.75, "valid context_pause_threshold passes through"); + } + + { + const { preferences } = validatePreferences({ models: { research: "claude-opus-4-6" } }); + assertEq(preferences.models?.research, "claude-opus-4-6", "valid models passes through"); + } + + { + const { preferences } = validatePreferences({ auto_supervisor: { model: "claude-opus-4-6" } }); + assertEq(preferences.auto_supervisor?.model, "claude-opus-4-6", "valid auto_supervisor passes through"); + } + + { + const { preferences } = validatePreferences({ notifications: { enabled: true } }); + assertEq(preferences.notifications?.enabled, true, "valid notifications passes through"); + } + + { + const { preferences } = validatePreferences({ remote_questions: { channel: "slack", channel_id: "C123" } }); + assertEq(preferences.remote_questions?.channel, "slack", "valid remote_questions passes through"); + } + + console.log("\n=== mixed valid/invalid/unknown keys ==="); + + { + const prefs = { + uat_dispatch: true, + totally_made_up: "value", + budget_ceiling: "garbage", + } as unknown as GSDPreferences; + const { preferences, errors, warnings } = validatePreferences(prefs); + + // Valid key works + assertEq(preferences.uat_dispatch, true, "valid uat_dispatch preserved"); + + // Unknown key warned + assertTrue(warnings.some(w => w.includes("totally_made_up")), "unknown key warned"); + + // Invalid value errored and dropped + assertTrue(errors.some(e => e.includes("budget_ceiling")), "invalid budget_ceiling errored"); + assertEq(preferences.budget_ceiling, undefined, "invalid budget_ceiling dropped"); + } + + console.log("\n=== existing behavior preserved ==="); + + // Ensure deprecated git fields still produce deprecation warnings (not unknown-key warnings) + { + const { warnings } = validatePreferences({ git: { isolation: "worktree" } } as GSDPreferences); + assertTrue(warnings.some(w => w.includes("deprecated")), "deprecated git.isolation still warns"); + const unknownWarnings = warnings.filter(w => w.includes("unknown")); + assertEq(unknownWarnings.length, 0, "git is a known key — no unknown-key warning"); + } + + report(); +} + +main();