diff --git a/src/selfhost/private-config.ts b/src/selfhost/private-config.ts index 7520a816c9..75c9669e4e 100644 --- a/src/selfhost/private-config.ts +++ b/src/selfhost/private-config.ts @@ -168,18 +168,34 @@ function reviewContextFolders(repoFullName: string): string[] { return [join(`${owner}__${repo}`, "review"), join(repo, "review")]; } +/** Read a `name:` / `when:` frontmatter value robustly. The value text (everything after the key on its line) is + * parsed as a standalone YAML scalar, so the real parser handles quoting, escaped `\"` / doubled `''` quotes, and a + * trailing inline comment — `"SQL #1 Rubric"` keeps its internal `#`, `SQL Rubric # note` drops the comment. A + * value the YAML parser rejects standalone — notably an unquoted glob that begins with a `*` wildcard — falls back + * to a lenient strip (drop an inline comment and any surrounding quote) so those globs keep working. */ +function reviewSkillScalar(rawValue: string): string { + try { + const parsed = parseYaml(rawValue); + if (typeof parsed === "string") return parsed.trim(); + } catch { + // not a standalone-parseable scalar (e.g. an unquoted *-leading glob) — fall through to the lenient strip + } + return rawValue.replace(/\s+#.*$/, "").replace(/^["']|["']$/g, "").trim(); +} + /** Parse a skill markdown file into {name, when, body}. YAML frontmatter (`---\nname:\nwhen:\n---`) is optional; name - * defaults to the filename and `when` to "always". */ + * defaults to the filename and `when` to "always". `name`/`when` are decoded through the YAML parser (see + * reviewSkillScalar) so a quoted value keeps its contents (incl. an internal `#`) while a trailing inline comment is + * dropped — an unstripped comment corrupts the label and turns `when` into a glob that never matches, silently + * disabling the rubric. */ export function parseReviewSkill(filename: string, text: string): RepoReviewSkill { const fm = /^---\s*\n([\s\S]*?)\n---\s*\n?([\s\S]*)$/.exec(text); const head = fm?.[1] ?? ""; const body = (fm?.[2] ?? text).trim(); - // Strip surrounding quotes on `name` too, symmetric with `when` below — a quoted scalar - // (`name: "SQL Rubric"`) is ordinary YAML frontmatter, so the quotes must not survive into the label. - const nameRaw = /(?:^|\n)name:\s*(.+)/.exec(head)?.[1]?.trim(); - const name = (nameRaw ?? "").replace(/^["']|["']$/g, "") || filename.replace(/\.md$/i, ""); - const whenRaw = /(?:^|\n)when:\s*(.+)/.exec(head)?.[1]?.trim(); - const when = (whenRaw ?? "always").replace(/^["']|["']$/g, "") || "always"; + const nameRaw = /(?:^|\n)name:\s*(.+)/.exec(head)?.[1]; + const name = (nameRaw !== undefined ? reviewSkillScalar(nameRaw) : "") || filename.replace(/\.md$/i, ""); + const whenRaw = /(?:^|\n)when:\s*(.+)/.exec(head)?.[1]; + const when = (whenRaw !== undefined ? reviewSkillScalar(whenRaw) : "always") || "always"; return { name, when, body }; } diff --git a/test/unit/private-config.test.ts b/test/unit/private-config.test.ts index cbd505be81..92f14eb082 100644 --- a/test/unit/private-config.test.ts +++ b/test/unit/private-config.test.ts @@ -226,6 +226,25 @@ describe("parseReviewSkill (#review-skills)", () => { expect(parseReviewSkill("y.md", "---\nname: 'Voice Guide'\n---\nb").name).toBe("Voice Guide"); expect(parseReviewSkill("fallback.md", '---\nname: ""\n---\nb').name).toBe("fallback"); }); + it("ignores a trailing YAML inline comment on name and when, quote-aware", () => { + // A trailing ` # …` is a YAML comment, not part of the scalar: left in, it corrupts the label and turns + // `when` into a glob that never matches, silently disabling the rubric. + expect(parseReviewSkill("sql.md", '---\nname: SQL Rubric # the sql one\nwhen: "**/*.sql" # only sql\n---\nBody.\n')).toEqual({ name: "SQL Rubric", when: "**/*.sql", body: "Body." }); + // A `#` with no preceding whitespace is part of the value (real YAML), not a comment — must be preserved. + expect(parseReviewSkill("cs.md", '---\nname: "C# Rubric"\n---\nb').name).toBe("C# Rubric"); + expect(parseReviewSkill("z.md", "---\nname: a#b\n---\nb").name).toBe("a#b"); + // A `#` INSIDE a quoted scalar — even with preceding whitespace — is part of the value, not a comment. + expect(parseReviewSkill("h.md", '---\nname: "SQL #1 Rubric"\nwhen: "src/#hot/**" # trailing note\n---\nb')).toEqual({ name: "SQL #1 Rubric", when: "src/#hot/**", body: "b" }); + // YAML-escaped double quotes and doubled single quotes are decoded, not treated as the terminator. + expect(parseReviewSkill("e.md", '---\nname: "SQL \\"Index\\" Rubric"\n---\nb').name).toBe('SQL "Index" Rubric'); + expect(parseReviewSkill("o.md", "---\nname: 'Owner''s Rubric'\n---\nb").name).toBe("Owner's Rubric"); + // An unquoted *-leading glob is not valid standalone YAML; it must still survive as the literal when-glob. + expect(parseReviewSkill("g.md", "---\nwhen: **/*.ts\n---\nb").when).toBe("**/*.ts"); + // A non-string YAML scalar (e.g. a bare number) falls through to the literal text rather than a typed value. + expect(parseReviewSkill("n.md", "---\nname: 42\n---\nb").name).toBe("42"); + // A malformed unterminated quote degrades to stripping the stray leading quote (back-compat, not a crash). + expect(parseReviewSkill("u.md", '---\nname: "unterminated\n---\nb').name).toBe("unterminated"); + }); }); describe("isReviewSkillEnabled (#review-skills)", () => {