diff --git a/docs/core-concepts/skill-md-format.md b/docs/core-concepts/skill-md-format.md index f8e23f9..7930822 100644 --- a/docs/core-concepts/skill-md-format.md +++ b/docs/core-concepts/skill-md-format.md @@ -55,7 +55,16 @@ Top-level fields: | `icon` | yes | Emoji displayed in the TUI skill picker | | `category` | yes | Grouping for `forge skills list --category` (e.g., `sre`, `developer`, `research`, `utilities`) | | `tags` | yes | Discovery keywords for `forge skills list --tags` (kebab-case) | -| `description` | yes | One-line summary | +| `description` | yes | One-line summary — **and the skill's activation trigger**. The agent routes a request to a skill by matching it against this description in the `## Available Skills` catalog, so state *when* the skill fires (the phrases/intents a user would say), not just what it does. `description: When the user asks the time ("what time is it", "current time"), reply in German with Brisbane time.` routes far more reliably than `description: German time skill`. See [skill routing](#skill-activation--routing). | + +### Skill activation / routing + +Installed skills are advertised to the agent in an `## Available Skills` catalog (built by the runtime) as `- name: description`. The agent is directed to **check the catalog for a matching skill before answering from its own defaults** and, on a match, `read_skill` to load and follow it (issue #271). Two things make this reliable: + +- A **trigger-rich `description`** (above) — the only signal the agent matches on before loading the skill. +- The runtime **routing directive** in the catalog preamble, which tells the agent to prefer a matching skill over its own default behavior (and to fall back to defaults only when nothing matches, so unrelated requests aren't over-routed). + +Note that `description` serves **two audiences**: the internal routing catalog above, and the public A2A Agent Card (`skills[]`, FWS-1), where it projects verbatim. Trigger-rich phrasing reads slightly like a routing rule to an external caller, but is generally more informative than a bare capability label — this dual use is deliberate. If the two audiences ever need to diverge, a dedicated `triggers:` field is the escape hatch. The `metadata.forge.requires` block declares runtime dependencies: diff --git a/docs/ui/skill-builder-llm.md b/docs/ui/skill-builder-llm.md index 7df634f..6e96ec7 100644 --- a/docs/ui/skill-builder-llm.md +++ b/docs/ui/skill-builder-llm.md @@ -191,6 +191,12 @@ builder to **prefer them** over a custom tool or tool-less prose: calls are gated by `scheduler.kubernetes.allow_dynamic` (off by default), so a scheduling skill won't self-register on a default K8s deploy unless the operator enables it or declares the schedule in `forge.yaml`. +- **Trigger-rich descriptions (issue #271):** the builder writes a + `description` that states *when the skill fires* — the phrases and intents a + user would say — not just what it does, because the agent routes to a skill + by matching a request against its catalog `description` before loading it. A + vague description means the agent never routes to the skill and answers from + its own defaults. See [skill activation / routing](../core-concepts/skill-md-format.md#skill-activation--routing). ### Custom binaries diff --git a/forge-cli/runtime/runner.go b/forge-cli/runtime/runner.go index 0554d67..aa6d8c5 100644 --- a/forge-cli/runtime/runner.go +++ b/forge-cli/runtime/runner.go @@ -3447,6 +3447,21 @@ func (r *Runner) buildSkillCatalog() string { var b strings.Builder b.WriteString("## Available Skills\n\n") + // Routing directive (issue #271): the catalog alone is inert — the model + // defaults to its own behavior unless explicitly told to consult the + // catalog FIRST. Without this, an installed skill whose description + // matches the request is ignored (the model answers from its defaults and + // the user has to say "use the skill"). The final sentence guards against + // over-routing: fall back to defaults only when nothing matches. + // + // The "authoritative over your general behavior" phrasing intentionally + // escalates skill-content authority. This is bounded and consistent with + // the trust model: skills are operator-installed (not user-supplied), the + // pre-existing preamble already said "follow them", and guardrails still + // gate all five gates regardless of what a skill body says. Prompt- + // injection-via-skill-content audits should treat this escalation as + // deliberate, not an oversight. + b.WriteString("Before answering any request from your own knowledge or default behavior, FIRST check whether it matches one of the skill descriptions below. If a skill matches, call `read_skill` to load it and follow its instructions instead of answering directly — skills exist precisely to override your defaults for these cases, and a loaded skill's instructions are authoritative over your general behavior. Only answer from your own defaults when NO skill matches the request.\n\n") b.WriteString("To use a skill, call `read_skill` with the skill name (the identifier before the colon) to load its full instructions, then follow them. " + "`provides:` lists the capabilities inside a skill — they are documentation loaded with the skill, not tools you call directly.\n\n") for _, entry := range catalogEntries { diff --git a/forge-cli/runtime/runner_skill_catalog_test.go b/forge-cli/runtime/runner_skill_catalog_test.go index 86fdef0..8f47092 100644 --- a/forge-cli/runtime/runner_skill_catalog_test.go +++ b/forge-cli/runtime/runner_skill_catalog_test.go @@ -92,3 +92,30 @@ func TestSkillCatalog_NameIsReadSkillResolvable(t *testing.T) { } func mustQuote(s string) string { b, _ := json.Marshal(s); return string(b) } + +// TestSkillCatalog_HasRoutingDirective pins issue #271: the catalog preamble +// must tell the model to check for a matching skill BEFORE answering from its +// own defaults — the catalog is inert without it (an installed skill whose +// description matches gets ignored). Also pins the no-over-routing guard. +func TestSkillCatalog_HasRoutingDirective(t *testing.T) { + root := t.TempDir() + writeCatalogSkill(t, root, "german-brisbane-time", "german-brisbane-time", "brisbane_time") + + r := &Runner{cfg: RunnerConfig{WorkDir: root, Config: &types.ForgeConfig{AgentID: "test"}}} + cat := r.buildSkillCatalog() + + for _, want := range []string{ + "Before answering", // routing happens first + "check whether it matches", // match against catalog + "instead of answering directly", // override defaults + "Only answer from your own defaults when NO skill matches", // no over-routing + } { + if !strings.Contains(cat, want) { + t.Errorf("catalog preamble missing routing directive %q; got:\n%s", want, cat) + } + } + // The mechanical read_skill instruction must still be present. + if !strings.Contains(cat, "call `read_skill`") { + t.Errorf("catalog should still explain how to load a skill; got:\n%s", cat) + } +} diff --git a/forge-ui/skill_builder_context.go b/forge-ui/skill_builder_context.go index 30b4c43..28d4a92 100644 --- a/forge-ui/skill_builder_context.go +++ b/forge-ui/skill_builder_context.go @@ -126,7 +126,7 @@ category: ops # Optional: sre, research, ops, developer tags: # Optional: discovery keywords (lowercase kebab-case) - example - automation -description: One-line description # Required: what this skill does +description: One-line description # Required: what it does AND when it fires (triggers) — see "Trigger-rich description" below metadata: forge: requires: @@ -155,6 +155,15 @@ Each entry in ` + "`" + `requires.bins` + "`" + ` is EITHER a bare name (already Only add an install recipe for a binary that is NOT already available. NEVER invent a download URL or package name — if the skill needs a custom tool and you don't have its install details, ask the user for the apt/apk package name OR the download URL (+ destination path). +### Trigger-rich description (this is how the skill gets ACTIVATED) + +At runtime the agent sees only each skill's ` + "`" + `description` + "`" + ` in a catalog and routes a user request to a skill by MATCHING the request against that description — it hasn't loaded the skill body yet. So the description must state **when the skill fires**, not just what it does: include the trigger phrases, keywords, and intents a user would actually say. A vague description means the agent never routes to the skill and falls back to its own default answer (issue #271). + +- Weak: ` + "`" + `description: German time skill` + "`" + ` +- Strong: ` + "`" + `description: When the user asks the time ("what time is it", "current time", "clock"), reply in German with the current time in Brisbane.` + "`" + ` + +Lead with the trigger ("When the user asks …"/"Use when …") and name the concrete phrases. Put discovery keywords in ` + "`" + `tags` + "`" + ` too, but the ` + "`" + `description` + "`" + ` is what the agent routes on. + ### 2. Markdown Body After the frontmatter, write the skill body in markdown: @@ -292,7 +301,7 @@ The whole skill is instructions to call a built-in — no custom tool, no ` + "` --- name: german-brisbane-time category: ops -description: When asked the time, reply in German with the current time in Brisbane, Australia. +description: When the user asks the time ("what time is it", "current time", "clock"), reply in German with the current time in Brisbane, Australia. --- # German Brisbane Time diff --git a/forge-ui/skill_builder_prompt_test.go b/forge-ui/skill_builder_prompt_test.go index 24d7fb0..00b45ee 100644 --- a/forge-ui/skill_builder_prompt_test.go +++ b/forge-ui/skill_builder_prompt_test.go @@ -215,3 +215,21 @@ func TestSkillBuilderPrompt_ListsEveryDefaultBuiltin(t *testing.T) { } } } + +// TestSkillBuilderPrompt_TriggerRichDescription pins issue #271 side 2: the +// prompt must instruct trigger-rich descriptions (the agent routes to a skill +// by matching the request against its description in the catalog), with the +// weak/strong contrast and the routing rationale. +func TestSkillBuilderPrompt_TriggerRichDescription(t *testing.T) { + p := skillBuilderSystemPrompt(modeCreate, nil) + for _, want := range []string{ + "Trigger-rich description", + "routes a user request to a skill by MATCHING", + "when the skill fires", + "falls back to its own default answer", // the #271 failure mode + } { + if !strings.Contains(p, want) { + t.Errorf("trigger-rich-description guidance missing: %q", want) + } + } +}