diff --git a/README.md b/README.md index edd62dd..1a9ba9b 100644 --- a/README.md +++ b/README.md @@ -268,6 +268,8 @@ Live cost $ and context % are tracked per run and surfaced in the Tiers view. Ov Invalid values and unknown keys produce a startup warning naming the file, the field, and the bad value — never a silent no-op. Absent files are normal. +Agent frontmatter `thinkingLevel` is validated with the same strictness (#90): an invalid value (typo'd level, wrong type) fails that agent's load with an actionable warning naming the file, the bad value, and the valid levels (`off | minimal | low | medium | high | xhigh | max`) — never a silent no-op. A YAML-empty value (`thinkingLevel:` with nothing after it) counts as absent. + ### Operational runtime `src/runtime/` — the async/scheduling spine: diff --git a/src/registry/frontmatter.ts b/src/registry/frontmatter.ts index 8d6c681..da73d84 100644 --- a/src/registry/frontmatter.ts +++ b/src/registry/frontmatter.ts @@ -5,6 +5,14 @@ import { basename, extname } from "node:path"; export type AgentSource = "builtin" | "project" | "global"; export type ThinkingLevel = "off" | "minimal" | "low" | "medium" | "high" | "xhigh" | "max"; +export const THINKING_LEVELS: readonly ThinkingLevel[] = ["off", "minimal", "low", "medium", "high", "xhigh", "max"]; + +/** Type guard for the closed ThinkingLevel enum. #90: canonical home is HERE, next to + * the type; fleet-settings imports + re-exports it for its settings-field guard. */ +export function isThinkingLevel(v: unknown): v is ThinkingLevel { + return typeof v === "string" && (THINKING_LEVELS as readonly string[]).includes(v); +} + export interface AgentDef { name: string; description: string; @@ -70,12 +78,27 @@ export function parseAgentFile(content: string, filePath: string, source: AgentS const backend = rawBackend as "pi" | "claude"; const sessionKey = typeof raw.sessionKey === "string" && raw.sessionKey.trim() ? raw.sessionKey.trim() : name; + // #90: thinkingLevel is a closed enum — same value class as backend, same contract: + // trim-then-validate, an invalid value throws FrontmatterError (discovery catches → + // warning + skip), never a silent no-op. `null` (YAML empty value) counts as absent. + const rawThinking: unknown = raw.thinkingLevel; + let thinkingLevel: ThinkingLevel | undefined; + if (rawThinking !== undefined && rawThinking !== null) { + const candidate = typeof rawThinking === "string" ? rawThinking.trim() : rawThinking; + if (!isThinkingLevel(candidate)) { + throw new FrontmatterError( + `${filePath}: invalid thinkingLevel ${JSON.stringify(rawThinking)} (must be one of ${THINKING_LEVELS.join("|")})`, + ); + } + thinkingLevel = candidate; + } + return { name, description, model: typeof raw.model === "string" ? raw.model : undefined, tier: typeof raw.tier === "string" ? raw.tier : undefined, - thinkingLevel: typeof raw.thinkingLevel === "string" ? (raw.thinkingLevel as ThinkingLevel) : undefined, + thinkingLevel, tools: strList(raw.tools), skills: strList(raw.skills), rolePrompt: body, diff --git a/src/settings/fleet-settings.ts b/src/settings/fleet-settings.ts index 10a1267..4bbe77b 100644 --- a/src/settings/fleet-settings.ts +++ b/src/settings/fleet-settings.ts @@ -14,13 +14,11 @@ // - The schema is intentionally small and additive; unknown keys warn so typos // surface instead of no-oping. import { readFileSync } from "node:fs"; -import type { ThinkingLevel } from "../registry/frontmatter.ts"; +import { THINKING_LEVELS, isThinkingLevel, type ThinkingLevel } from "../registry/frontmatter.ts"; -const THINKING_LEVELS: readonly ThinkingLevel[] = ["off", "minimal", "low", "medium", "high", "xhigh", "max"]; - -export function isThinkingLevel(v: unknown): v is ThinkingLevel { - return typeof v === "string" && (THINKING_LEVELS as readonly string[]).includes(v); -} +// #90: isThinkingLevel + THINKING_LEVELS moved to registry/frontmatter.ts (next to the +// canonical type). Re-exported here so existing importers keep working. +export { isThinkingLevel, THINKING_LEVELS }; /** Fleet-wide defaults. Intentionally additive — new fields land here. */ export interface FleetSettings { diff --git a/test/discovery.test.mts b/test/discovery.test.mts index 2bec76c..6fed244 100644 --- a/test/discovery.test.mts +++ b/test/discovery.test.mts @@ -58,4 +58,17 @@ test("missing dirs are tolerated (no throw)", () => { const r = discoverAgents({ projectDir: "/nonexistent", globalDir: "/nonexistent", builtinDir: null }); strictEqual(r.agents.size, 0); strictEqual(r.errors.length, 0); -}); \ No newline at end of file +}); +test("#90: invalid thinkingLevel file skipped + actionable warning, siblings load", () => { + const proj = mkdtempSync(join(tmpdir(), "proj5-")); + writeFileSync(join(proj, "bad.md"), "---\nname: bad\ndescription: d\nthinkingLevel: ultra\n---\nbody\n"); + writeFileSync(join(proj, "good.md"), agentFile("good")); + const r = discoverAgents({ projectDir: proj, globalDir: null, builtinDir: null }); + ok( + r.warnings.some((w) => w.includes("bad.md") && w.includes("invalid thinkingLevel")), + "actionable warning surfaced", + ); + ok(!r.agents.has("bad"), "invalid agent skipped"); + ok(r.agents.has("good"), "sibling loaded"); + rmSync(proj, { recursive: true, force: true }); +}); diff --git a/test/frontmatter.test.mts b/test/frontmatter.test.mts index 211c834..11775dc 100644 --- a/test/frontmatter.test.mts +++ b/test/frontmatter.test.mts @@ -1,6 +1,6 @@ // test/frontmatter.test.mts import { test } from "node:test"; -import { strictEqual, deepStrictEqual, throws } from "node:assert"; +import { strictEqual, deepStrictEqual, throws, ok } from "node:assert"; import { parseAgentFile } from "../src/registry/frontmatter.ts"; const BASE = `--- @@ -97,4 +97,59 @@ test("userMemory: true parses true", () => { test("userMemory: false parses false", () => { const a = parseAgentFile("---\nname: a\ndescription: d\nuserMemory: false\n---\nrole", "/tmp/agent.md", "builtin"); strictEqual(a.userMemory, false); -}); \ No newline at end of file +}); +// --- #90: thinkingLevel is a closed enum — invalid values throw (like backend), +// never silently no-op. Same value class, same contract. --- + +test("#90: invalid thinkingLevel string throws FrontmatterError with actionable message", () => { + const bad = "---\nname: g\ndescription: d\nthinkingLevel: ultra\n---\nbody\n"; + throws( + () => parseAgentFile(bad, "/x/g.md", "project"), + (e: unknown) => { + ok(e instanceof Error && e.name === "FrontmatterError", `FrontmatterError, got ${String(e)}`); + ok(e instanceof Error && e.message.includes("invalid thinkingLevel"), `names the field: ${e.message}`); + ok(e instanceof Error && e.message.includes("ultra"), `names the bad value: ${e.message}`); + ok(e instanceof Error && e.message.includes("off|minimal|low|medium|high|xhigh|max"), `lists valid levels: ${e.message}`); + return true; + }, + ); +}); + +test("#90: non-string thinkingLevel throws FrontmatterError", () => { + const bad = "---\nname: g\ndescription: d\nthinkingLevel: 5\n---\nbody\n"; + throws(() => parseAgentFile(bad, "/x/g.md", "project"), { name: "FrontmatterError" }); +}); + +test("#90: thinkingLevel absent → undefined (back-compat)", () => { + const a = parseAgentFile("---\nname: a\ndescription: d\n---\nrole", "/tmp/agent.md", "builtin"); + strictEqual(a.thinkingLevel, undefined); +}); + +test("#90: thinkingLevel null (YAML empty value) → treated as absent", () => { + const a = parseAgentFile("---\nname: a\ndescription: d\nthinkingLevel:\n---\nrole", "/tmp/agent.md", "builtin"); + strictEqual(a.thinkingLevel, undefined); +}); + +test("#90: all seven thinking levels parse", () => { + for (const level of ["off", "minimal", "low", "medium", "high", "xhigh", "max"] as const) { + const md = `---\nname: a\ndescription: d\nthinkingLevel: ${level}\n---\nrole\n`; + strictEqual(parseAgentFile(md, "/tmp/agent.md", "builtin").thinkingLevel, level, `level ${level}`); + } +}); + +test("#90: quoted padded value trims (symmetric with backend, review NIT 1)", () => { + const a = parseAgentFile("---\nname: a\ndescription: d\nthinkingLevel: \" low\"\n---\nrole", "/tmp/agent.md", "builtin"); + strictEqual(a.thinkingLevel, "low"); +}); + +test("#90: array value throws with the value rendered (review NIT 2)", () => { + const bad = "---\nname: g\ndescription: d\nthinkingLevel: [low]\n---\nbody\n"; + throws( + () => parseAgentFile(bad, "/x/g.md", "project"), + (e: unknown) => { + ok(e instanceof Error && e.name === "FrontmatterError", `FrontmatterError, got ${String(e)}`); + ok(e instanceof Error && e.message.includes('["low"]'), `renders the array: ${e.message}`); + return true; + }, + ); +});