diff --git a/apps/server/src/provider/Drivers/ClaudeSkills.test.ts b/apps/server/src/provider/Drivers/ClaudeSkills.test.ts index 60db1d0c5e26..079e51e84d36 100644 --- a/apps/server/src/provider/Drivers/ClaudeSkills.test.ts +++ b/apps/server/src/provider/Drivers/ClaudeSkills.test.ts @@ -219,6 +219,122 @@ it.layer(NodeServices.layer)("discoverClaudeSkills", (it) => { }), ); + it.effect( + "recovers a description containing an unquoted colon that Claude Code itself accepts", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const tempDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-claude-skills-" }); + const configDir = path.join(tempDir, "claude-home"); + + yield* writeSkill( + path.join(configDir, "skills"), + "kane-cli", + [ + "---", + "name: kane-cli", + "description: Browser automation + AI test authoring via kane-cli: run browser objectives, ...", + "---", + ].join("\n"), + ); + + const skills = yield* discoverClaudeSkills({ homePath: configDir }, undefined); + + assert.deepEqual(skills, [ + { + name: "kane-cli", + path: path.join(configDir, "skills", "kane-cli", "SKILL.md"), + enabled: true, + scope: "user", + description: + "Browser automation + AI test authoring via kane-cli: run browser objectives, ...", + }, + ]); + }), + ); + + it.effect("strips a trailing comment from a recovered value instead of keeping it verbatim", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const tempDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-claude-skills-" }); + const configDir = path.join(tempDir, "claude-home"); + + yield* writeSkill( + path.join(configDir, "skills"), + "commented", + [ + "---", + "name: demo # display label", + // The colon-containing description is what forces the lenient + // fallback to run at all — a comment alone wouldn't fail strict + // parsing, so this is needed to actually exercise the fallback's + // value parsing rather than the strict-YAML path. + "description: Browser automation + AI test authoring via kane-cli: run browser objectives, ...", + "---", + ].join("\n"), + ); + + const skills = yield* discoverClaudeSkills({ homePath: configDir }, undefined); + + assert.deepEqual(skills, [ + { + name: "demo", + path: path.join(configDir, "skills", "commented", "SKILL.md"), + enabled: true, + scope: "user", + description: + "Browser automation + AI test authoring via kane-cli: run browser objectives, ...", + }, + ]); + }), + ); + + it.effect("skips the whole skill when a broken field survives alongside a recoverable one", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const tempDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-claude-skills-" }); + const configDir = path.join(tempDir, "claude-home"); + + yield* writeSkill( + path.join(configDir, "skills"), + "broken-name", + ["---", "name: [unclosed", "description: Broken skill.", "---"].join("\n"), + ); + + const skills = yield* discoverClaudeSkills({ homePath: configDir }, undefined); + + // A broken `name` must not surface the skill under its directory name + // with only the description recovered — Claude Code wouldn't load + // this file at all. + assert.deepEqual(skills, []); + }), + ); + + it.effect("skips the whole skill when an unread field has broken YAML syntax", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const tempDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-claude-skills-" }); + const configDir = path.join(tempDir, "claude-home"); + + yield* writeSkill( + path.join(configDir, "skills"), + "broken-other-field", + ["---", "name: demo", "allowed-tools: [unclosed", "---"].join("\n"), + ); + + const skills = yield* discoverClaudeSkills({ homePath: configDir }, undefined); + + // The broken field isn't one this scanner reads, but it still means + // the document has a real YAML syntax error Claude Code would reject + // outright — recovering `name` in isolation would be wrong. + assert.deepEqual(skills, []); + }), + ); + it.effect("honors CLAUDE_CONFIG_DIR from the environment when homePath is unset", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/provider/Drivers/ClaudeSkills.ts b/apps/server/src/provider/Drivers/ClaudeSkills.ts index 5c33fba0b9e9..3e83a166bcbe 100644 --- a/apps/server/src/provider/Drivers/ClaudeSkills.ts +++ b/apps/server/src/provider/Drivers/ClaudeSkills.ts @@ -30,20 +30,82 @@ type SkillFrontmatter = | { readonly kind: "malformed" } | { readonly kind: "parsed"; readonly name?: string; readonly description?: string }; +// A YAML flow/block construct starting here means the author was attempting +// real YAML nesting that broke, not a plain scalar that merely contains a +// colon — the lenient recovery below must not paper over that. +const YAML_STRUCTURAL_VALUE_PATTERN = /^[[{|>&*!]/; + +/** + * Recovers `name`/`description` as flat "key: value" scalars when strict YAML + * parsing rejects the frontmatter. Claude Code's own frontmatter parser is + * more lenient than a real YAML parser — it accepts an unquoted scalar + * description containing a "word: " sequence (e.g. "... via kane-cli: run + * ..."), which strict YAML rejects as an ambiguous nested mapping. A skill + * that demonstrably loads in Claude Code must not be invisible in T3 purely + * over that mismatch. This only recovers the two scalar fields we read, and + * only when no top-level line's value looks like broken YAML syntax (an + * unterminated flow collection, block scalar, anchor, alias, or tag) — any + * such line, in this or another field, means the document has a real syntax + * error Claude Code wouldn't load either, so the whole file counts as + * malformed rather than surfacing a partial, plausible-looking recovery. + */ +function parseSkillFrontmatterLeniently(yamlSource: string): SkillFrontmatter { + const fields: Partial> = {}; + for (const rawLine of yamlSource.split(/\r?\n/)) { + if (/^\s/.test(rawLine) || rawLine.trim().length === 0) { + // Indented (nested/continuation) or blank lines aren't a top-level + // scalar this recovery can safely reinterpret. + continue; + } + const separatorIndex = rawLine.indexOf(":"); + if (separatorIndex === -1) { + continue; + } + const key = rawLine.slice(0, separatorIndex).trim(); + const rawValue = rawLine.slice(separatorIndex + 1).trim(); + if (YAML_STRUCTURAL_VALUE_PATTERN.test(rawValue)) { + return { kind: "malformed" }; + } + if (key !== "name" && key !== "description") { + continue; + } + // Parse the value in isolation with the real YAML parser rather than + // just trimming it: a plain scalar with a trailing "# comment" or one + // quoted with escapes needs real YAML scalar rules to come out right. + // The one case this is *for* — an unquoted value with its own embedded + // ": " — parses as a one-entry mapping in isolation too, not a string, + // so it correctly falls through to the untouched raw text below. + let value = rawValue; + try { + const parsedValue: unknown = parseYamlDocument(rawValue); + if (typeof parsedValue === "string") { + value = parsedValue; + } + } catch { + // Not parseable in isolation either; keep the raw text. + } + if (value.length > 0) { + fields[key] = value; + } + } + return Object.keys(fields).length > 0 ? { kind: "parsed", ...fields } : { kind: "malformed" }; +} + function parseSkillFrontmatter(contents: string): SkillFrontmatter { const match = FRONTMATTER_PATTERN.exec(contents); if (!match) { return { kind: "missing" }; } + const yamlSource = match[1] ?? ""; let parsed: unknown; try { - parsed = parseYamlDocument(match[1] ?? ""); + parsed = parseYamlDocument(yamlSource); } catch { - return { kind: "malformed" }; + return parseSkillFrontmatterLeniently(yamlSource); } if (typeof parsed !== "object" || parsed === null) { - return { kind: "malformed" }; + return parseSkillFrontmatterLeniently(yamlSource); } const record = parsed as Record;