feat(skills): accept paths, validate SKILL.md, and bound eager injection (#350)

CHANGELOG.md

@@ -9,6 +9,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **`skills:` now accepts filesystem paths, not just built-in names**
+  (issue #350) — an entry is treated as a path when it starts with `.`
+  or `~`, or contains `/` or `\`; everything else must still be
+  a registered built-in, so a bare `conductor` can never be shadowed by a
+  same-named local directory. A path may point at a single skill directory
+  (one holding `SKILL.md`) or at a root of them, which expands to every
+  immediate child that holds one. Relative paths resolve against the workflow
+  file's directory — the same rule `working_dir` uses — so a team can version
+  a skill alongside the workflow that uses it with no per-developer install
+  step, and the workflow resolves identically from any working directory.
+  Conductor expands roots itself rather than handing them to a provider,
+  because eager injection needs a name per skill and `claude-agent-sdk` needs
+  a `<plugin>:<skill>` name; doing it centrally keeps every provider seeing
+  the same set. Skill paths are trusted input by design: the same workflow
+  file can already declare `type: script` steps running arbitrary shell, so
+  no additional allowlist applies.
+- **`runtime.skill_injection` bounds eagerly injected skill content**
+  (issue #350) — `warn_bytes` (default 64KB) logs a warning and reports from
+  `conductor validate`; `max_bytes` (default 128KB) fails the agent. Either
+  can be set to `null` to disable it. Providers without a native skill
+  surface (`claude`, `hermes`) have no progressive disclosure: `AgentExecutor`
+  prepends each enabled skill's `SKILL.md` **plus its entire `references/`
+  tree** on every call and every retry, and there
+  was previously no ceiling at all. The bundled `conductor` skill alone is
+  ~117KB (~29K tokens), so the defaults deliberately straddle it — enabling
+  it on `claude` now warns instead of breaking, while accumulating several
+  large skills errors. Both limits are measured against the exact string
+  being prepended and report a per-skill breakdown naming the offender.
+  Providers with progressive disclosure (`copilot`, `claude-agent-sdk`) are
+  unaffected.
+- **`hermes` declares `skills=True`** (issue #350) — the provider omitted
+  `skills` from its `CAPABILITIES`, which defaults to `False`, so
+  `conductor validate` rejected `skills:` on it while its own `execute()`
+  docstring described eager injection working. Injection happens in
+  `AgentExecutor`, upstream of every provider, so the path was always
+  reachable and the declaration was simply inaccurate. Now bounded by
+  `runtime.skill_injection` like `claude`.
+
 - **`claude-agent-sdk` provider now honors `working_dir`** — the directory
   resolved from `agent.working_dir` / `runtime.working_dir` is forwarded to
   `ClaudeAgentOptions.cwd`, so the `claude` CLI runs there and every stdio MCP
@@ -81,6 +119,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **A malformed `SKILL.md` no longer fails silently** (issue #350) — both the
+  Copilot CLI and Claude Code skip a skill whose YAML frontmatter cannot be
+  parsed, with no warning and no error, leaving an agent running without the
+  knowledge its author asked for. The trap is ordinary: a `description`
+  containing `Triggers: ...` as an unquoted plain scalar is invalid YAML.
+  Conductor now parses the frontmatter itself, requires a non-empty `name`
+  and `description`, and reports the underlying YAML error along with the
+  `description: |` block-scalar fix. Enforced during resolution rather than
+  only in `conductor validate`, because `conductor run` never invokes the
+  static validator.
+- **`conductor validate` rejects a `claude-agent-sdk` skill outside a plugin**
+  (issue #350) — that SDK exposes no bare skill-directory option, only plugin
+  roots plus skill names, so such a skill is unreachable there even though
+  `copilot` loads it fine. It previously surfaced as a runtime
+  `ProviderError` on first execution; it is now reported before the run
+  starts, naming the directory and offering both remedies (package it as a
+  plugin, or run the agent on `copilot`).
+
 - **`skills: []` is now a real opt-out on `claude-agent-sdk`, and agents no
   longer inherit ambient skills from the machine.** The provider left the SDK's
   `setting_sources` unset, so the `claude` CLI discovered and enabled skills
@@ -141,8 +197,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   every prompt.** The provider previously took the eager preamble path on the
   grounds that the SDK had no skill surface — out of date, and expensive: the
   full `SKILL.md` plus the entire `references/` tree was prepended to every
-  call, every retry, and every validator pass (~27K tokens for the bundled
-  `conductor` skill). The owning Claude Code plugin is now registered on the
+  call and every retry (~29K tokens for the bundled `conductor` skill). The owning Claude Code plugin is now registered on the
   session and the skill enabled by its `<plugin>:<skill>` name, so the CLI reads
   only the frontmatter up front and loads the body on demand. An agent with an
   explicit `tools: []` is granted back the single `Skill` tool when it has

docs/providers/comparison.md

@@ -157,7 +157,18 @@ the environment instead.
 Note the SDK treats the enabled-skill list as a context filter rather than a sandbox:
 undeclared skills are hidden from the model's listing and rejected by the `Skill`
 tool, but their files remain readable on disk through `Read`/`Bash`.
-- `temperature` and `max_tokens` are **rejected at the factory** — sampling behavior is controlled by the CLI.
+
+Because the SDK exposes **no bare skill-directory option** — a skill is enabled by
+name through the plugin that ships it — a skill directory that is not inside a
+Claude Code plugin cannot be loaded here at all. `conductor validate` reports such
+a `skills:` entry before the run starts, naming the directory and both remedies:
+package it as a plugin (a `.claude-plugin/plugin.json` with the skill under
+`<plugin>/skills/`), or run that agent on `copilot`, which registers skill
+directories directly and accepts the identical skill untouched. See the
+[Skills section of the workflow syntax guide](../workflow-syntax.md#skills).
+
+Separately, `temperature` and `max_tokens` are **rejected at the factory** —
+sampling behavior is controlled by the CLI.
 
 ### Example Claude Agent SDK Workflow
 

docs/workflow-syntax.md

@@ -11,6 +11,7 @@ This document provides a comprehensive reference for the Conductor workflow YAML
 - [Inputs and Outputs](#inputs-and-outputs)
 - [Limits and Safety](#limits-and-safety)
 - [Tools](#tools)
+- [Skills](#skills)
 - [External File References](#external-file-references)
 - [Hooks](#hooks)
 
@@ -1419,6 +1420,156 @@ agents:
 
 For full MCP configuration details, see the [MCP Tools guide](mcp-tools.md).
 
+## Skills
+
+A **skill** is a directory of reusable knowledge an agent can opt into: a
+`SKILL.md` describing what the skill covers, plus an optional `references/`
+tree of supporting docs. Conductor consumes the same format the GitHub
+Copilot CLI and Anthropic Claude Code use, so a skill written for either
+generally works here unchanged. Conductor adds two constraints neither CLI
+enforces: the frontmatter must actually parse (that is the point of this —
+see below), and on `claude-agent-sdk` the frontmatter `name` must match the
+directory basename, since the skill is enabled by name.
+
+### Enabling skills
+
+Set a workflow-wide default and override it per agent:
+
+```yaml
+workflow:
+  runtime:
+    skills:
+      - conductor                     # built-in, ships in the wheel
+      - ./team-skills/acme-widgets    # versioned alongside the workflow
+      - ~/scratch/skills              # a skills root — every skill directly inside
+
+agents:
+  - name: reviewer
+    prompt: "Review this workflow."
+    # inherits runtime.skills
+
+  - name: summarizer
+    prompt: "Summarize the review."
+    skills: []                        # explicit opt-out — no skills
+
+  - name: widget_expert
+    prompt: "Check the widget conventions."
+    skills: [./team-skills/acme-widgets]   # overrides the workflow default
+```
+
+The per-agent field is tri-state:
+
+| Value | Meaning |
+|---|---|
+| omitted | inherit `runtime.skills` |
+| `[]` | explicit opt-out — no skills, ignores the workflow default |
+| `[...]` | explicit set — replaces the workflow default |
+
+Skills apply only to provider-backed agents. They are rejected on `script`,
+`wait`, `set`, `terminate`, `workflow`, and `human_gate` steps.
+
+### Names and paths
+
+Each entry is either a **registered built-in name** or a **filesystem path**.
+The distinction is syntactic — an entry is a path when it starts with `.` or
+`~`, or contains `/` or `\`. Everything else must be a
+built-in name, so a bare `conductor` can never be shadowed by a directory
+that happens to share its name.
+
+Conductor ships one built-in skill:
+
+| Name | Contents |
+|---|---|
+| `conductor` | Conductor's YAML schema, execution model, authoring patterns, and CLI commands |
+
+A path may point at either granularity:
+
+* a **skill directory** — one containing `SKILL.md`
+* a **skills root** — a directory of skill directories, which expands to
+  every immediate child containing a `SKILL.md` (not recursive)
+
+Relative paths resolve against the **workflow file's directory**, the same
+rule `working_dir` uses, so a workflow validates and runs identically from
+any working directory. This is what lets a team version a skill next to the
+workflow that uses it, with no per-developer install step.
+
+> **Trust:** a `SKILL.md` is injected into the agent's context, so treat
+> skill paths as trusted input. Conductor applies no additional allowlist —
+> the same workflow file can already declare `type: script` steps that run
+> arbitrary shell, so a skill path grants strictly less.
+
+### `SKILL.md` frontmatter
+
+Every resolved skill must declare a `name` and a `description` in valid YAML
+frontmatter:
+
+```yaml
+---
+name: acme-widgets
+description: |
+  Internal ACME widget conventions. Triggers: widget, acme widget.
+---
+```
+
+Use a block scalar (`description: |`) whenever the text contains a colon
+followed by a space — without one the value is invalid YAML, and this is the
+single most common mistake:
+
+```yaml
+# Wrong — 'Triggers:' makes this unparseable
+description: Internal ACME widget conventions. Triggers: widget, acme widget.
+```
+
+Both the Copilot CLI and Claude Code **silently skip** a skill whose
+frontmatter fails to parse — no warning, no error, the skill is simply
+absent. Conductor parses it itself and fails loudly instead, both at
+`conductor validate` and at run time.
+
+### How skills reach the model
+
+The contract is the same everywhere — *the agent has access to the named
+skill* — but the mechanism and its cost differ:
+
+| Provider | Mechanism | Cost |
+|---|---|---|
+| `copilot` | `skill_directories` on the SDK session | progressive — frontmatter only, body loaded on demand |
+| `claude-agent-sdk` | owning plugin registered, skill enabled by `<plugin>:<skill>` | progressive |
+| `claude` | eager injection into the rendered prompt | **full body on every call** |
+| `hermes` | eager injection into the rendered prompt | **full body on every call** |
+| `aca` | not supported (`skills=False`) — rejected by `conductor validate` and by the executor at run time | n/a |
+
+Two consequences worth knowing:
+
+* **`claude-agent-sdk` requires a plugin.** The SDK has no bare
+  skill-directory option, so a skill must live inside a Claude Code plugin
+  (a `.claude-plugin/plugin.json` with the skill under `<plugin>/skills/`).
+  `conductor validate` reports a path skill that is not, rather than letting
+  it fail mid-run. The same skill works on `copilot` untouched.
+* **Eager injection is expensive.** The bundled `conductor` skill alone is
+  ~117KB (~29K tokens), prepended to *every* call and every retry.
+
+### Limiting eager injection
+
+`runtime.skill_injection` bounds what eager-injection providers prepend. It
+has no effect on providers with progressive disclosure.
+
+```yaml
+workflow:
+  runtime:
+    skill_injection:
+      warn_bytes: 65536      # Default: 65536 (64KB). null disables the warning.
+      max_bytes: 131072      # Default: 131072 (128KB). null disables the limit.
+```
+
+Exceeding `warn_bytes` logs a warning and reports it from `conductor
+validate`; exceeding `max_bytes` fails the agent. Both are measured against
+the exact string being prepended and report a per-skill breakdown, so the
+offender is named. The defaults sit either side of the bundled `conductor`
+skill: enabling it on `claude` warns rather than breaking, while
+accumulating several large skills errors.
+
+See `examples/skills-self-improving-workflow.yaml` for a complete example.
+
 ## External File References
 
 The `!file` YAML tag lets you reference external files from any YAML field value. The file content is transparently inlined during loading, keeping workflow files concise and enabling reuse of prompts, schemas, and configuration across workflows.

examples/skills-self-improving-workflow.yaml

@@ -16,14 +16,26 @@
 #     by its `<plugin>:<skill>` name.
 #   * Claude: SKILL.md + references/*.md are eagerly prepended to the
 #     agent's rendered prompt inside <skills><skill name="conductor">
-#     ... </skill></skills> tags.
+#     ... </skill></skills> tags. That is ~117KB (~29K tokens) for this
+#     skill, paid on every call and retry — `runtime.skill_injection`
+#     bounds it.
 #
 # Tri-state opt-in via list presence:
 #   * Omit `skills:`     → inherit `runtime.skills`
 #   * `skills: []`       → explicit opt-out (e.g. the `formatter` agent
 #                          below, which doesn't need workflow knowledge)
 #   * `skills: [name]`   → explicit set, replaces the workflow default
 #
+# Entries may also be filesystem paths, not just built-in names — see
+# `docs/workflow-syntax.md` (Skills). A path starts with . or ~, or
+# contains / or \; it resolves against THIS FILE's directory, and may
+# point at a single skill directory or a root of them:
+#
+#   runtime:
+#     skills:
+#       - conductor                     # built-in, ships in the wheel
+#       - ./team-skills/acme-widgets    # versioned next to the workflow
+#
 # Usage:
 #   conductor run examples/skills-self-improving-workflow.yaml \
 #     --input task="Write a workflow that summarises a GitHub issue."

plugins/conductor/skills/conductor/references/authoring.md

@@ -220,6 +220,18 @@ See `examples/validator.yaml` for a complete example.
 
 `skills` enables reusable knowledge or capability bundles for provider-backed agents. The Conductor distribution ships one built-in skill — `conductor` — which packages the YAML schema, execution model, and authoring patterns (the same content this reference doc covers) so an agent can evaluate, improve, debug, or generate Conductor workflows.
 
+**Names and paths.** Each entry is either a registered built-in name or a filesystem path. The distinction is syntactic: an entry is a path when it starts with `.` or `~`, or contains `/` or `\` — everything else must be a built-in name, so a bare `conductor` is never shadowed by a same-named local directory. A path points at either a single skill directory (one holding `SKILL.md`) or a root of them, which expands to every immediate child holding one (not recursive). Relative paths resolve against the **workflow file's directory**, the same rule `working_dir` uses, so a skill can be versioned alongside the workflow with no per-developer install step and the workflow behaves identically from any working directory. Skill paths are trusted input — no allowlist applies, since the same file can already run arbitrary shell via `type: script`.
+
+**`SKILL.md` frontmatter.** Every resolved skill must declare a non-empty `name` and `description` in valid YAML frontmatter. Use a block scalar whenever the text contains a colon followed by a space — `description: Does things. Triggers: a, b` is invalid YAML, and both the Copilot CLI and Claude Code skip such a skill *silently*. Conductor parses it and fails loudly instead, at validate time and at run time:
+
+```yaml
+---
+name: acme-widgets
+description: |
+  Internal ACME widget conventions. Triggers: widget, acme widget.
+---
+```
+
 **Tri-state per-agent field (resolved via list presence):**
 - Omit `skills:` — inherit from `runtime.skills`
 - `skills: []` — explicit opt-out (no skills for this agent, regardless of workflow default)
@@ -230,18 +242,27 @@ See `examples/validator.yaml` for a complete example.
 **Provider mechanism (same observable contract — "the agent has access to the named skill"):**
 - **Copilot** — the resolved skill directory is registered on the SDK session via `skill_directories`, so the agent discovers and loads skill content natively (progressive disclosure via `SKILL.md` frontmatter). This is more token-efficient than eager injection.
 - **Claude Agent SDK** — also native, through the Claude Code plugin surface: the plugin owning the skill is registered on the session and the skill enabled by its `<plugin>:<skill>` name. Skills the workflow did not declare are suppressed, so `skills: []` really is an opt-out and ambient skills from the machine never load.
-- **Claude** — the loader reads `SKILL.md` plus every `references/*.md` file in the skill directory and prepends them to the agent's rendered prompt inside `<skills><skill name="...">...</skill></skills>` tags. Inserted between workspace instructions and the user prompt.
+- **Claude** and **hermes** — the loader reads `SKILL.md` plus every `references/*.md` file in the skill directory and prepends them to the agent's rendered prompt inside `<skills><skill name="...">...</skill></skills>` tags. Inserted between workspace instructions and the user prompt.
+
+**Two provider-specific limits worth knowing:**
+- `claude-agent-sdk` has **no bare skill-directory option** — a skill is enabled by name through the plugin that ships it. A path skill outside a Claude Code plugin is therefore rejected at validation time, with both remedies named (package it as a plugin, or run that agent on `copilot`, which accepts the identical skill untouched).
+- Eager injection has no progressive disclosure: the whole body is prepended on every call and every retry. The bundled `conductor` skill alone is ~117KB (~29K tokens). `runtime.skill_injection` bounds it — `warn_bytes` (default 64KB) warns, `max_bytes` (default 128KB) fails the agent, either can be `null` to disable. Providers with progressive disclosure are unaffected.
 
-Not allowed on `script`, `human_gate`, `workflow`, `wait`, `set`, or `terminate` agent types. Unknown skill names fail at workflow validation time.
+Not allowed on `script`, `human_gate`, `workflow`, `wait`, `set`, or `terminate` agent types. Unknown built-in names fail when the config loads; unresolvable paths and malformed `SKILL.md` files fail at workflow validation time and again at run time.
 
 ```yaml
 workflow:
   runtime:
-    skills: [conductor]             # all provider-backed agents get the conductor skill
+    skills:
+      - conductor                   # built-in: ships in the wheel
+      - ./team-skills/acme-widgets  # path: versioned next to this workflow
+    skill_injection:                # only bounds eager-injection providers
+      warn_bytes: 65536
+      max_bytes: 131072
 
 agents:
   - name: workflow_reviewer
-    skills: [conductor]             # per-agent opt-in (redundant here, kept for clarity)
+    skills: [conductor]             # explicit set, replaces the workflow default
     prompt: "Review this workflow for correctness..."
 
   - name: simple_agent