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) <noreply@anthropic.com>
This commit is contained in:
TÂCHES
GitHub
parent
bcd3c9209c
commit
06f4bdc7f4
2 changed files with 275 additions and 1 deletions
|
|
@ -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<string>([
|
||||
"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[] = [];
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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();
|
||||
Loading…
Add table
Reference in a new issue