From bff94b2ada96feec9f8cbb905f2be44e5b278090 Mon Sep 17 00:00:00 2001 From: Fini Date: Sun, 19 Apr 2026 19:06:35 +0800 Subject: [PATCH] fix(ai): gate elements skill behind hasMcpTools flag (no prompt pollution) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex stop-hook #8: elements.md had `trigger: null` which makes resolveSkills('generation', ...) unconditionally load its 1500-token N-tool reference into every generation prompt — including the embedded orchestrator in apps/web/src/services/ai that emits single-shot JSON and CANNOT call MCP tools. The content was dead weight there (orchestrator-sub-agent.ts:333 / ai-prompts.ts:132 both build generation prompt via resolveSkills without any tool-use path). Fix: trigger: { flags: [hasMcpTools] }. Skill only auto-loads when caller explicitly declares MCP tools are available. No existing caller sets this flag, so the embedded orchestrator prompt is now clean again. External MCP clients (Claude Code / Codex / Gemini CLI / Cursor) still get the content via get_design_prompt(section='elements'), which uses getSkillByName direct lookup and bypasses resolveSkills' trigger filter. That contract is preserved. Added explanatory HTML comment in the skill header so future editors understand the gating + opt-in rule. Tests: 4 new cases in design-prompt-elements.test.ts verify: - getSkillByName returns skill regardless of flags (direct lookup) - resolveSkills('generation') WITHOUT flag → elements excluded - resolveSkills('generation') WITH {hasMcpTools:true} → elements included - buildDesignPrompt('elements') works regardless of flag (bypass path) 14/14 design-prompt-elements tests pass; 186/186 across pen-mcp + pen-ai-skills. format + tsc green. Bundle rebuilt. --- .../skills/phases/generation/elements.md | 19 ++++++++++- .../__tests__/design-prompt-elements.test.ts | 34 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/pen-ai-skills/skills/phases/generation/elements.md b/packages/pen-ai-skills/skills/phases/generation/elements.md index 355664b1c..6a4280514 100644 --- a/packages/pen-ai-skills/skills/phases/generation/elements.md +++ b/packages/pen-ai-skills/skills/phases/generation/elements.md @@ -2,12 +2,29 @@ name: elements description: N-tool element family reference — when and how to call add_card_row_v0 / add_metric_row_v0 / add_nav_chip_row_v0 / add_bottom_nav_v0 / add_activity_ring_v0 instead of hand-building via batch_design phase: [generation] -trigger: null +trigger: + flags: [hasMcpTools] priority: 14 budget: 1500 category: base --- + + ELEMENT TOOLS (schema-constrained alternatives to batch_design): These narrow MCP tools emit well-known structures that batch_design frequently gets wrong on non-Claude models (overflow, wrong role, anti-pattern layout). Each is shape-locked — you pick the tool by matching intent, then supply only content. Visual styling (color, font) stays orthogonal: override via a follow-up batch_design U-op if needed. diff --git a/packages/pen-mcp/src/__tests__/design-prompt-elements.test.ts b/packages/pen-mcp/src/__tests__/design-prompt-elements.test.ts index 7e5d16933..a1f61b4fc 100644 --- a/packages/pen-mcp/src/__tests__/design-prompt-elements.test.ts +++ b/packages/pen-mcp/src/__tests__/design-prompt-elements.test.ts @@ -10,6 +10,7 @@ */ import { describe, it, expect } from 'vitest'; +import { getSkillByName, resolveSkills } from '@zseven-w/pen-ai-skills'; import { buildDesignPrompt, listPromptSections } from '../tools/design-prompt'; import { DESIGN_TOOL_DEFINITIONS } from '../routes/design-routes'; @@ -82,3 +83,36 @@ describe('get_design_prompt — elements section', () => { expect(unknown).toBe(full); }); }); + +describe('elements skill — flag-gated auto-loading (no prompt pollution)', () => { + it('getSkillByName("elements") returns the skill regardless of flags (direct lookup)', () => { + const skill = getSkillByName('elements'); + expect(skill).toBeDefined(); + expect(skill?.meta.name).toBe('elements'); + // Trigger is a flag gate, not unconditional loading + expect(skill?.meta.trigger).toEqual({ flags: ['hasMcpTools'] }); + }); + + it('resolveSkills("generation") WITHOUT hasMcpTools flag → elements NOT auto-included', () => { + const ctx = resolveSkills('generation', 'create a dashboard', { flags: {} }); + const names = ctx.skills.map((s) => s.meta.name); + expect(names).not.toContain('elements'); + }); + + it('resolveSkills("generation") WITH hasMcpTools flag → elements IS auto-included', () => { + const ctx = resolveSkills('generation', 'create a dashboard', { + flags: { hasMcpTools: true }, + }); + const names = ctx.skills.map((s) => s.meta.name); + expect(names).toContain('elements'); + }); + + it('buildDesignPrompt("elements") returns content regardless of flags (uses direct lookup, not resolver)', () => { + // get_design_prompt's section path bypasses resolveSkills — it uses + // getSkillByName directly. So the flag gate doesn't affect this path: + // external MCP clients asking for the section explicitly still get it. + const content = buildDesignPrompt('elements'); + expect(content.length).toBeGreaterThan(500); + expect(content).toContain('add_card_row_v0'); + }); +});