feat(selfhost): let a review skill opt out via enabled: false frontmatter (#2590)

* feat(selfhost): let a review skill opt out via `enabled: false` frontmatter

Add isReviewSkillEnabled(text): a self-host review skill can set `enabled: false`
(or no/off/0) in its frontmatter to be omitted from the review context, so an
operator can turn a rubric off without deleting the file. The local
review-context reader skips disabled skills. Fully backward-compatible: a skill
without an `enabled` key (every existing one) stays enabled, so behavior is
unchanged unless the directive is explicitly set. Cover the parse vocabulary and
the reader-level omission.

* fix(selfhost): ignore YAML inline comment on review-skill enabled directive

`enabled: true # note` captured the whole value tail (`true # note`),
failed the truthy test, and wrongly disabled the skill. Strip a trailing
` # …` comment before matching so an inline-commented directive reads as
its bare value.

Author: ultrahighsuper

src/selfhost/private-config.ts

@@ -110,10 +110,22 @@ export function parseReviewSkill(filename: string, text: string): RepoReviewSkil
   return { name, when, body };
 }
 
+/** True unless a skill's frontmatter explicitly disables it with `enabled: false` (or `no`/`off`/`0`). Absent or
+ *  truthy `enabled` keeps the skill, so existing skills are unaffected — this only lets an operator turn a rubric
+ *  OFF without deleting the file. Truthy vocabulary matches the codebase flag convention. (#review-skills) */
+export function isReviewSkillEnabled(text: string): boolean {
+  const head = /^---\s*\n([\s\S]*?)\n---\s*\n?/.exec(text)?.[1] ?? "";
+  // Drop a YAML inline comment (` # …`) before matching, so `enabled: true  # explicit` reads as `true`, not
+  // `true # explicit` (which would fail the truthy test and wrongly disable the skill).
+  const raw = /(?:^|\n)enabled:\s*(.+)/.exec(head)?.[1]?.replace(/\s+#.*$/, "").trim().replace(/^["']|["']$/g, "");
+  return raw === undefined ? true : /^(1|true|yes|on)$/i.test(raw);
+}
+
 /** Build the container-local review-context reader over GITTENSORY_REPO_CONFIG_DIR, or null when the dir is unset. Per
  *  repo (first existing folder wins) reads `review/AGENTS.md` (Codex) or `review/CLAUDE.md` (Claude Code) as the
- *  guide + every `review/skills/*.md` rubric module, sorted. Missing files/dir degrade to nulls/empty; a per-file
- *  read error skips that file. (#review-skills) */
+ *  guide + every `review/skills/*.md` rubric module, sorted. A skill whose frontmatter sets `enabled: false` is
+ *  omitted (turned off without deleting the file). Missing files/dir degrade to nulls/empty; a per-file read
+ *  error skips that file. (#review-skills) */
 export function makeLocalReviewContextReader(dir: string | undefined): RepoReviewContextReader | null {
   const trimmed = (dir ?? "").trim();
   if (!trimmed) return null;
@@ -135,7 +147,9 @@ export function makeLocalReviewContextReader(dir: string | undefined): RepoRevie
         const entries = (await readdir(resolve(abs, "skills"))).filter((f) => f.toLowerCase().endsWith(".md")).sort();
         for (const f of entries) {
           try {
-            skills.push(parseReviewSkill(f, await readFile(resolve(abs, "skills", f), "utf8")));
+            const text = await readFile(resolve(abs, "skills", f), "utf8");
+            if (!isReviewSkillEnabled(text)) continue; // `enabled: false` frontmatter disables a skill without deleting it
+            skills.push(parseReviewSkill(f, text));
           } catch {
             // unreadable skill file → skip it
           }

test/unit/private-config.test.ts

@@ -2,7 +2,7 @@ import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { dirname, join } from "node:path";
 import { describe, expect, it } from "vitest";
-import { GLOBAL_CONFIG_CANDIDATES, localConfigCandidates, makeLocalManifestReader, makeLocalReviewContextReader, parseReviewSkill } from "../../src/selfhost/private-config";
+import { GLOBAL_CONFIG_CANDIDATES, isReviewSkillEnabled, localConfigCandidates, makeLocalManifestReader, makeLocalReviewContextReader, parseReviewSkill } from "../../src/selfhost/private-config";
 import { loadRepoReviewContext, setLocalReviewContextReader } from "../../src/signals/focus-manifest-loader";
 
 describe("localConfigCandidates (container-private config paths)", () => {
@@ -125,6 +125,24 @@ describe("parseReviewSkill (#review-skills)", () => {
   });
 });
 
+describe("isReviewSkillEnabled (#review-skills)", () => {
+  it("keeps a skill by default and honors an explicit enabled directive", () => {
+    expect(isReviewSkillEnabled("no frontmatter at all")).toBe(true); // no frontmatter → enabled
+    expect(isReviewSkillEnabled("---\nname: x\n---\nbody")).toBe(true); // frontmatter without `enabled` → enabled
+    expect(isReviewSkillEnabled("---\nenabled: true\n---\nbody")).toBe(true);
+    expect(isReviewSkillEnabled('---\nenabled: "on"\n---\nbody')).toBe(true); // quoted truthy stripped
+    expect(isReviewSkillEnabled("---\nenabled: false\n---\nbody")).toBe(false);
+    expect(isReviewSkillEnabled("---\nname: x\nenabled: no\n---\nbody")).toBe(false);
+    expect(isReviewSkillEnabled("---\nenabled: 0\n---\nbody")).toBe(false); // any non-truthy value disables
+  });
+  it("ignores a YAML inline comment on the enabled directive", () => {
+    // A trailing ` # …` is a YAML comment, not part of the value — it must not flip a truthy directive to disabled.
+    expect(isReviewSkillEnabled("---\nenabled: true # temporarily explicit\n---\nbody")).toBe(true);
+    expect(isReviewSkillEnabled('---\nenabled: "on"  # keep the rubric on\n---\nbody')).toBe(true); // comment after quoted value
+    expect(isReviewSkillEnabled("---\nenabled: false # parked for now\n---\nbody")).toBe(false); // still disables
+  });
+});
+
 describe("makeLocalReviewContextReader (#review-skills)", () => {
   it("returns null when the dir is unset/blank", () => {
     expect(makeLocalReviewContextReader(undefined)).toBeNull();
@@ -147,6 +165,16 @@ describe("makeLocalReviewContextReader (#review-skills)", () => {
     expect(ctx.skills.map((s) => s.name)).toEqual(["a-first", "second"]); // sorted by filename; .txt ignored
   });
 
+  it("omits a skill whose frontmatter sets enabled: false", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "gt-review-"));
+    const rev = join(dir, "jsonbored__gittensory", "review");
+    mkdirSync(join(rev, "skills"), { recursive: true });
+    writeFileSync(join(rev, "skills", "a-active.md"), "---\nname: active\nwhen: always\n---\nActive rubric.\n");
+    writeFileSync(join(rev, "skills", "b-disabled.md"), "---\nname: disabled\nenabled: false\n---\nParked rubric.\n");
+    const ctx = await makeLocalReviewContextReader(dir)!("JSONbored/gittensory");
+    expect(ctx.skills.map((s) => s.name)).toEqual(["active"]); // the disabled skill is dropped, not deleted
+  });
+
   it("falls back to legacy CLAUDE.md in the bare repo-name folder; returns empty for a missing or invalid repo", async () => {
     const dir = mkdtempSync(join(tmpdir(), "gt-review-"));
     mkdirSync(join(dir, "metagraphed", "review"), { recursive: true });