From d3b580805d6d869582bfc038217ed637dc36dee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Andr=C3=A9s=20Leiva=20Castillo?= Date: Mon, 29 Jun 2026 21:30:06 -0600 Subject: [PATCH 1/2] feat(skills): detect ambiguous-trigger verbs in bundled skill descriptions Bundled PostHog analytics skills whose descriptions contain coding-overlap verbs (investigate, debug, explore, etc.) without an explicit TRIGGER guard bias the model toward analytics skill invocations during unrelated coding sessions. This adds static analysis to catch and enforce those guards. Co-Authored-By: Claude Sonnet 4.6 --- biome.jsonc | 12 ++ .../core/src/skills/analyzeSkills.test.ts | 108 ++++++++++- packages/core/src/skills/analyzeSkills.ts | 41 ++++- .../src/skills/bundled-skill-corpus.test.ts | 174 ++++++++++++++++++ 4 files changed, 333 insertions(+), 2 deletions(-) create mode 100644 packages/core/src/skills/bundled-skill-corpus.test.ts diff --git a/biome.jsonc b/biome.jsonc index f2bf55073a..caf730ceb2 100644 --- a/biome.jsonc +++ b/biome.jsonc @@ -209,6 +209,18 @@ } } }, + { + // Core test files run in Node.js — relax the node:* restriction for tests only. + // Production code in packages/core must never import node:* (enforced above). + "includes": ["packages/core/src/**/*.test.ts"], + "linter": { + "rules": { + "style": { + "noRestrictedImports": "off" + } + } + } + }, { "includes": [ "packages/api-client/src/**/*.ts", diff --git a/packages/core/src/skills/analyzeSkills.test.ts b/packages/core/src/skills/analyzeSkills.test.ts index 2fd1aa5549..c915c87b25 100644 --- a/packages/core/src/skills/analyzeSkills.test.ts +++ b/packages/core/src/skills/analyzeSkills.test.ts @@ -1,6 +1,11 @@ import type { SkillInfo } from "@posthog/shared"; import { describe, expect, it } from "vitest"; -import { analyzeSkills, OVERSIZED_SKILL_MD_BYTES } from "./analyzeSkills"; +import { + AMBIGUOUS_TRIGGER_VERBS, + analyzeSkills, + hasAmbiguousTrigger, + OVERSIZED_SKILL_MD_BYTES, +} from "./analyzeSkills"; function makeSkill(overrides: Partial): SkillInfo { return { @@ -133,6 +138,107 @@ describe("analyzeSkills", () => { }); }); + describe("ambiguous-trigger", () => { + it("flags a bundled skill with an ambiguous verb and no TRIGGER guard", () => { + const skill = makeSkill({ + source: "bundled", + description: "Investigate why a metric dropped using PostHog data.", + }); + + const analysis = analyzeSkills([skill]); + + expect(analysis[skill.path]).toEqual([ + expect.objectContaining({ type: "ambiguous-trigger" }), + ]); + }); + + it.each(["user", "repo", "marketplace", "codex"] as const)( + "does not flag a %s skill — guard requirement is bundled-only", + (source) => { + const skill = makeSkill({ + source, + description: "Investigate why a metric dropped.", + }); + + expect(analyzeSkills([skill])).toEqual({}); + }, + ); + + it("does not flag a bundled skill whose description has a TRIGGER guard", () => { + const skill = makeSkill({ + source: "bundled", + description: + "Investigate the experiment results.\nTRIGGER when: user provides an experiment ID.\nDO NOT TRIGGER when: user is editing code.", + }); + + expect(analyzeSkills([skill])).toEqual({}); + }); + + it.each([ + [ + "Use when", + "Investigates a PostHog error. Use when the user pastes an issue URL.", + ], + [ + "Read when", + "Audit PostHog flags. Read when the user asks to health-check flags.", + ], + [ + "TRIGGER when", + "Investigate traces.\nTRIGGER when: user pastes a trace URL.", + ], + [ + "trigger when (lowercase)", + "Investigate traces. ONLY trigger when the user pastes a trace URL.", + ], + ])( + "does not flag a bundled skill with a %s guard", + (_label, description) => { + const skill = makeSkill({ source: "bundled", description }); + expect(analyzeSkills([skill])).toEqual({}); + }, + ); + + it("does not flag a bundled skill with no ambiguous verbs", () => { + const skill = makeSkill({ + source: "bundled", + description: "Run an A/B test experiment in PostHog.", + }); + + expect(analyzeSkills([skill])).toEqual({}); + }); + + it("reports the matched verbs in the message", () => { + const skill = makeSkill({ + source: "bundled", + description: "Debug and diagnose why the dashboard is slow.", + }); + + const [issue] = analyzeSkills([skill])[skill.path] ?? []; + + expect(issue?.message).toContain("debug"); + expect(issue?.message).toContain("diagnose"); + }); + + it("hasAmbiguousTrigger returns false for a skill that is already guarded", () => { + const skill = makeSkill({ + source: "bundled", + description: + "Investigate the issue.\nTRIGGER when: user says investigate.", + }); + + expect(hasAmbiguousTrigger(skill)).toBe(false); + }); + + it("AMBIGUOUS_TRIGGER_VERBS covers the core coding-overlap verbs", () => { + // Sentinel: ensures the verb list is not accidentally emptied. + expect(AMBIGUOUS_TRIGGER_VERBS.length).toBeGreaterThanOrEqual(5); + expect(AMBIGUOUS_TRIGGER_VERBS).toContain("investigate"); + expect(AMBIGUOUS_TRIGGER_VERBS).toContain("debug"); + expect(AMBIGUOUS_TRIGGER_VERBS).toContain("diagnose"); + }); + }); + it("accumulates multiple issues on one skill", () => { const skill = makeSkill({ name: "Other Name", diff --git a/packages/core/src/skills/analyzeSkills.ts b/packages/core/src/skills/analyzeSkills.ts index f376671240..8834f48845 100644 --- a/packages/core/src/skills/analyzeSkills.ts +++ b/packages/core/src/skills/analyzeSkills.ts @@ -4,7 +4,8 @@ export type SkillIssueType = | "missing-description" | "name-mismatch" | "oversized-manifest" - | "shadowed"; + | "shadowed" + | "ambiguous-trigger"; export interface SkillIssue { type: SkillIssueType; @@ -17,6 +18,34 @@ export type SkillAnalysis = Record; /** SKILL.md is injected into agent context; warn when it gets expensive. */ export const OVERSIZED_SKILL_MD_BYTES = 32 * 1024; +/** + * Verbs that appear in both PostHog-analytics descriptions and everyday coding + * requests. A bundled skill whose description contains one of these without an + * explicit TRIGGER guard risks being invoked when the user is doing unrelated + * coding work — the model picks the closest-matching skill even when none is + * appropriate. + */ +export const AMBIGUOUS_TRIGGER_VERBS = [ + "investigate", + "diagnose", + "debug", + "explore", + "analyze", + "audit", + "instrument", + "set up", +] as const; + +export function hasAmbiguousTrigger(skill: SkillInfo): boolean { + if (skill.source !== "bundled") return false; + const desc = skill.description.toLowerCase(); + const hasVerb = AMBIGUOUS_TRIGGER_VERBS.some((v) => desc.includes(v)); + // Accepted guard patterns (any is sufficient): + // "TRIGGER when" / "trigger when", "Use when", "Read when", "DO NOT TRIGGER" + const hasGuard = /\b(trigger|use|read) when\b/i.test(skill.description); + return hasVerb && !hasGuard; +} + /** * Precedence when two skills share a name: the most specific source wins. * Repo skills beat user skills beat marketplace plugins beat bundled ones. @@ -64,6 +93,16 @@ export function analyzeSkills(skills: SkillInfo[]): SkillAnalysis { }); } + if (hasAmbiguousTrigger(skill)) { + const matched = AMBIGUOUS_TRIGGER_VERBS.filter((v) => + skill.description.toLowerCase().includes(v), + ); + push(skill, { + type: "ambiguous-trigger", + message: `Description uses generic verb(s) [${matched.join(", ")}] without a TRIGGER guard — add "TRIGGER when: …\\nDO NOT TRIGGER when: …" to prevent false invocations on coding tasks`, + }); + } + const dirName = directoryName(skill.path); if (skill.name !== dirName) { push(skill, { diff --git a/packages/core/src/skills/bundled-skill-corpus.test.ts b/packages/core/src/skills/bundled-skill-corpus.test.ts new file mode 100644 index 0000000000..e9e66c531b --- /dev/null +++ b/packages/core/src/skills/bundled-skill-corpus.test.ts @@ -0,0 +1,174 @@ +/** + * Corpus quality tests for the bundled PostHog plugin skills. + * + * These run against the real skill files that ship with the app and assert + * structural properties that prevent overfitting: being steered toward a + * PostHog analytics skill when the user is doing unrelated coding work. + * + * Tests SKIP when the bundled skills directory doesn't exist (fresh checkout + * before `pnpm build`). Once built, every failure names a skill that needs + * a TRIGGER guard in its description. + */ + +import * as fs from "node:fs"; +import * as path from "node:path"; +import type { SkillInfo } from "@posthog/shared"; +import { describe, expect, it } from "vitest"; +import { + AMBIGUOUS_TRIGGER_VERBS, + analyzeSkills, + hasAmbiguousTrigger, +} from "./analyzeSkills"; + +const BUNDLED_SKILLS_DIR = path.resolve( + __dirname, + "../../../../apps/code/.vite/build/plugins/posthog/skills", +); + +function loadBundledSkills(): SkillInfo[] | null { + if (!fs.existsSync(BUNDLED_SKILLS_DIR)) return null; + const entries = fs.readdirSync(BUNDLED_SKILLS_DIR, { withFileTypes: true }); + const skills: SkillInfo[] = []; + + for (const entry of entries) { + if (!entry.isDirectory() && !entry.isSymbolicLink()) continue; + const skillPath = path.join(BUNDLED_SKILLS_DIR, entry.name); + const manifestPath = path.join(skillPath, "SKILL.md"); + if (!fs.existsSync(manifestPath)) continue; + + const content = fs.readFileSync(manifestPath, "utf-8"); + const nameMatch = content.match(/^name:\s*(.+)$/m); + // Handles four YAML description forms: + // folded: description: >\n line\n line + // double: description: "text" + // single: description: 'text' + // plain: description: text on one line + const foldedMatch = content.match( + /^description:\s*>-?\s*\n((?:[ \t]+.+\n?)+)/m, + ); + const quotedMatch = content.match( + /^description:\s*(?:"([\s\S]+?)"|'([\s\S]+?)')\s*$/m, + ); + const plainMatch = content.match(/^description:\s+([^>'"|\n].+)$/m); + const rawDesc = foldedMatch + ? foldedMatch[1] + ?.split("\n") + .map((l) => l.trim()) + .join(" ") + .trim() + : quotedMatch + ? (quotedMatch[1] ?? quotedMatch[2] ?? "").trim() + : (plainMatch?.[1] ?? "").trim(); + + skills.push({ + name: nameMatch?.[1]?.trim() ?? entry.name, + description: rawDesc, + source: "bundled", + path: skillPath, + editable: false, + skillMdBytes: Buffer.byteLength(content, "utf-8"), + }); + } + + return skills.length > 0 ? skills : null; +} + +const skills = loadBundledSkills(); + +describe.skipIf(skills === null)("bundled posthog skill corpus", () => { + // TypeScript cannot narrow `skills` inside the callback even though + // describe.skipIf guarantees the suite only runs when skills !== null. + const corpus = skills as SkillInfo[]; + + it("loads a non-trivial number of skills (sanity check)", () => { + expect(corpus.length).toBeGreaterThan(50); + }); + + it("every skill has a non-empty description", () => { + const missing = corpus.filter((s) => !s.description.trim()); + expect(missing.map((s) => s.name)).toEqual([]); + }); + + /** + * Overfitting guard: every bundled skill whose description uses a + * coding-overlap verb must have an explicit TRIGGER guard. + * + * Without a guard the model sees "investigate why a metric dropped" and + * "investigate why a test is failing" as equally close to the same set + * of skills — picking a PostHog analytics skill for coding work. + * + * Fix: add to the skill's description field — + * TRIGGER when: + * DO NOT TRIGGER when: + */ + it("every ambiguous-verb skill has a TRIGGER guard in its description", () => { + const violations = corpus.filter(hasAmbiguousTrigger); + + if (violations.length > 0) { + const lines = violations.map((s) => { + const matched = AMBIGUOUS_TRIGGER_VERBS.filter((v) => + s.description.toLowerCase().includes(v), + ); + return ` ${s.name} [${matched.join(", ")}]`; + }); + console.error( + `${violations.length} skill(s) need a TRIGGER guard:\n${lines.join("\n")}`, + ); + } + + expect(violations.map((s) => s.name)).toEqual([]); + }); + + /** + * Intent-collision guard: no action verb should appear in more than 8 + * *unguarded* descriptions. Skills that already have a Use-when or TRIGGER + * guard are self-disambiguating — the model picks between them on domain + * specifics. Unguarded skills sharing the same verb give the model no basis + * to choose and cause false invocations in coding sessions. + */ + it("no action verb dominates more than 8 unguarded skill descriptions", () => { + const collisions: string[] = []; + + for (const word of AMBIGUOUS_TRIGGER_VERBS) { + const unguarded = corpus.filter( + (s) => + s.description.toLowerCase().includes(word) && hasAmbiguousTrigger(s), + ); + if (unguarded.length > 8) { + collisions.push( + `"${word}" in ${unguarded.length} unguarded descriptions: ${unguarded.map((s) => s.name).join(", ")}`, + ); + } + } + + if (collisions.length > 0) { + console.error( + `Intent collisions:\n${collisions.map((c) => ` ${c}`).join("\n")}`, + ); + } + + expect(collisions).toEqual([]); + }); + + it("analyzeSkills reports no issues on the full corpus", () => { + const analysis = analyzeSkills(corpus); + const allIssues = Object.entries(analysis).flatMap(([p, issues]) => + issues.map((issue) => ({ + skill: path.basename(p), + type: issue.type, + message: issue.message, + })), + ); + + if (allIssues.length > 0) { + console.error( + "Corpus issues:\n" + + allIssues + .map((i) => ` [${i.type}] ${i.skill}: ${i.message}`) + .join("\n"), + ); + } + + expect(allIssues).toEqual([]); + }); +}); From c5760eae8984fe064228bab2cec8de37c1812645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Andr=C3=A9s=20Leiva=20Castillo?= Date: Mon, 29 Jun 2026 21:42:07 -0600 Subject: [PATCH 2/2] fix(skills): extend trigger guard to when(ever), split sentinel test per it.each - Address Greptile review: fix comment on hasAmbiguousTrigger (was listing "DO NOT TRIGGER" as an accepted guard but the regex requires "when" after it), and extend the regex to also accept "Use whenever" / "TRIGGER whenever" as valid guards (debugging-surveys uses this phrasing). - Address Greptile review: split the AMBIGUOUS_TRIGGER_VERBS sentinel into a length check plus an it.each for the core verb set, per team convention. Co-Authored-By: Claude Sonnet 4.6 --- packages/core/src/skills/analyzeSkills.test.ts | 17 ++++++++++++----- packages/core/src/skills/analyzeSkills.ts | 6 ++++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/core/src/skills/analyzeSkills.test.ts b/packages/core/src/skills/analyzeSkills.test.ts index c915c87b25..db5af1286a 100644 --- a/packages/core/src/skills/analyzeSkills.test.ts +++ b/packages/core/src/skills/analyzeSkills.test.ts @@ -179,6 +179,10 @@ describe("analyzeSkills", () => { "Use when", "Investigates a PostHog error. Use when the user pastes an issue URL.", ], + [ + "Use whenever", + "Debug PostHog Surveys. Use whenever a support ticket is pasted.", + ], [ "Read when", "Audit PostHog flags. Read when the user asks to health-check flags.", @@ -230,13 +234,16 @@ describe("analyzeSkills", () => { expect(hasAmbiguousTrigger(skill)).toBe(false); }); - it("AMBIGUOUS_TRIGGER_VERBS covers the core coding-overlap verbs", () => { - // Sentinel: ensures the verb list is not accidentally emptied. + it("AMBIGUOUS_TRIGGER_VERBS contains at least 5 verbs (sentinel)", () => { expect(AMBIGUOUS_TRIGGER_VERBS.length).toBeGreaterThanOrEqual(5); - expect(AMBIGUOUS_TRIGGER_VERBS).toContain("investigate"); - expect(AMBIGUOUS_TRIGGER_VERBS).toContain("debug"); - expect(AMBIGUOUS_TRIGGER_VERBS).toContain("diagnose"); }); + + it.each(["investigate", "debug", "diagnose"] as const)( + "AMBIGUOUS_TRIGGER_VERBS includes the core coding-overlap verb %s", + (verb) => { + expect(AMBIGUOUS_TRIGGER_VERBS).toContain(verb); + }, + ); }); it("accumulates multiple issues on one skill", () => { diff --git a/packages/core/src/skills/analyzeSkills.ts b/packages/core/src/skills/analyzeSkills.ts index 8834f48845..15c11ea2d4 100644 --- a/packages/core/src/skills/analyzeSkills.ts +++ b/packages/core/src/skills/analyzeSkills.ts @@ -41,8 +41,10 @@ export function hasAmbiguousTrigger(skill: SkillInfo): boolean { const desc = skill.description.toLowerCase(); const hasVerb = AMBIGUOUS_TRIGGER_VERBS.some((v) => desc.includes(v)); // Accepted guard patterns (any is sufficient): - // "TRIGGER when" / "trigger when", "Use when", "Read when", "DO NOT TRIGGER" - const hasGuard = /\b(trigger|use|read) when\b/i.test(skill.description); + // "TRIGGER when(ever)", "Use when(ever)", "Read when(ever)", "DO NOT TRIGGER when" + const hasGuard = /\b(trigger|use|read) when(ever)?\b/i.test( + skill.description, + ); return hasVerb && !hasGuard; }