Commit fb06786
Changed files (6)
lib
elelem
skills
spec
elelem
skills
lib/elelem/skills/catalog.rb
@@ -4,15 +4,26 @@ module Elelem
module Skills
class Catalog
GEM_SKILLS = File.expand_path("builtin", __dir__).freeze
+ REMOTE_SKILLS = File.expand_path("~/.agents/remote-skills").freeze
+ # Order matters: later entries shadow earlier ones by skill name.
+ # remote_skills (see #initialize) is scanned immediately after
+ # GEM_SKILLS is scanned (see #discover), so the precedence is:
+ # bundled < remote-fetched < user-level < project-level. A skill
+ # fetched via `/skills fetch` never overrides one the user placed
+ # directly in ~/.agents/skills or .agents/skills. Note the remote
+ # scan is keyed on GEM_SKILLS being present in load_paths -- passing
+ # a load_paths without it (as tests do, to stay fixture-isolated)
+ # skips the remote scan too.
LOAD_PATHS = [
GEM_SKILLS,
"~/.agents/skills",
".agents/skills"
].freeze
- def initialize(load_paths: LOAD_PATHS)
+ def initialize(load_paths: LOAD_PATHS, remote_skills: REMOTE_SKILLS)
@load_paths = load_paths
+ @remote_skills = remote_skills
end
def all
@@ -35,22 +46,42 @@ module Elelem
seen = {}
@load_paths.each do |path|
- dir = File.expand_path(path)
- next unless File.directory?(dir)
-
- Dir["#{dir}/*/SKILL.md"].sort.each do |skill_md|
- skill = SkillFile.load(skill_md)
- next unless skill
-
- seen[skill.name] = skill
- rescue => e
- Elelem.logger.warn("elelem-skills: failed to load #{skill_md}: #{e.message}")
- end
+ scan_one_level(path, seen)
+ scan_remote(seen) if path == GEM_SKILLS && @remote_skills
end
seen.values
end
+ # Bundled and local skills: <root>/<skill>/SKILL.md
+ def scan_one_level(path, seen)
+ dir = File.expand_path(path)
+ return unless File.directory?(dir)
+
+ Dir["#{dir}/*/SKILL.md"].sort.each { |skill_md| load_into(skill_md, seen) }
+ end
+
+ # Fetched skills, partitioned by index host: <root>/<host>/<skill>/SKILL.md.
+ # Deliberately not Dir["**/SKILL.md"] -- an unbounded recursive glob
+ # would also pick up any SKILL.md nested arbitrarily deep inside a
+ # fetched skill's own resources (or a stray file entirely unrelated
+ # to this gem living under the same tree).
+ def scan_remote(seen)
+ dir = File.expand_path(@remote_skills)
+ return unless File.directory?(dir)
+
+ Dir["#{dir}/*/*/SKILL.md"].sort.each { |skill_md| load_into(skill_md, seen) }
+ end
+
+ def load_into(skill_md, seen)
+ skill = SkillFile.load(skill_md)
+ return unless skill
+
+ seen[skill.name] = skill
+ rescue => e
+ Elelem.logger.warn("elelem-skills: failed to load #{skill_md}: #{e.message}")
+ end
+
def truncate(text, limit)
return text if text.length <= limit
lib/elelem/skills/fetcher.rb
@@ -13,26 +13,35 @@ module Elelem
FetchError = Class.new(StandardError)
- def initialize(install_dir: File.expand_path("~/.agents/skills"), http: Net::HTTP)
+ def initialize(install_dir: Catalog::REMOTE_SKILLS, http: Net::HTTP)
@install_dir = install_dir
@http = http
end
# Fetches https://<origin>/.well-known/agent-skills/index.json, verifies
- # it, and installs every type: "skill-md" entry into install_dir. Entries
- # with an unsupported type (currently anything but "skill-md") are
- # skipped with a warning, per the spec's guidance for unrecognized types.
+ # it, and installs every type: "skill-md" entry into
+ # install_dir/<index host>/<skill name>/SKILL.md. Partitioning by the
+ # index's own host (not the artifact URL's host, which the spec allows
+ # to point elsewhere, e.g. a CDN) keeps two publishers who happen to
+ # both name a skill "wrangler" from clobbering each other on disk --
+ # Catalog still resolves by frontmatter name across all of them, so a
+ # same-named skill from two hosts is still ambiguous at that level;
+ # this only fixes the fetch-time file collision, not that ambiguity.
+ # Entries with an unsupported type (currently anything but "skill-md")
+ # are skipped with a warning, per the spec's guidance.
#
# Returns the list of skill names installed.
def fetch(origin)
- index_url = URI.join(normalize(origin), "/.well-known/agent-skills/index.json")
+ base = normalize(origin)
+ index_url = URI.join(base, "/.well-known/agent-skills/index.json")
index = get_json(index_url)
unless index["$schema"] == SCHEMA
raise FetchError, "unrecognized index schema #{index["$schema"].inspect} at #{index_url} (expected #{SCHEMA})"
end
- (index["skills"] || []).filter_map { |entry| install(entry, index_url) }
+ host_dir = File.join(@install_dir, sanitize(base.host))
+ (index["skills"] || []).filter_map { |entry| install(entry, index_url, host_dir) }
end
private
@@ -42,7 +51,11 @@ module Elelem
URI.parse(origin)
end
- def install(entry, index_url)
+ def sanitize(host)
+ host.to_s.downcase.gsub(/[^a-z0-9.-]/, "_")
+ end
+
+ def install(entry, index_url, host_dir)
name, type, url, digest = entry.values_at("name", "type", "url", "digest")
unless type == "skill-md"
@@ -53,7 +66,7 @@ module Elelem
body = get_raw(URI.join(index_url, url))
verify!(name, body, digest)
- dir = File.join(@install_dir, name)
+ dir = File.join(host_dir, sanitize(name))
FileUtils.mkdir_p(dir)
File.write(File.join(dir, "SKILL.md"), body)
lib/elelem/skills/setup.rb
@@ -62,7 +62,7 @@ module Elelem
end
names = Fetcher.new.fetch(origin)
- agent.output.say(names.empty? ? "No skills installed from #{origin}." : "Installed: #{names.join(", ")}\nRun /reload to pick them up.")
+ agent.output.say(names.empty? ? "No skills installed from #{origin}." : "Installed: #{names.join(", ")}\nRestart elelem to pick them up (/reload does not currently re-register this gem's tool -- see README).")
rescue Fetcher::FetchError => e
agent.output.say(e.message, as: :error)
end
spec/elelem/skills/catalog_spec.rb
@@ -49,6 +49,68 @@ RSpec.describe Elelem::Skills::Catalog do
end
end
+ describe "REMOTE_SKILLS" do
+ it "discovers skills fetched into <remote_skills>/<host>/<name>/SKILL.md" do
+ Dir.mktmpdir do |remote_root|
+ skill_dir = File.join(remote_root, "example.com", "wrangler")
+ FileUtils.mkdir_p(skill_dir)
+ File.write(File.join(skill_dir, "SKILL.md"), <<~SKILL)
+ ---
+ name: wrangler
+ description: fetched from example.com
+ ---
+ body
+ SKILL
+
+ catalog = described_class.new(load_paths: [described_class::GEM_SKILLS], remote_skills: remote_root)
+
+ expect(catalog.find("wrangler")&.description).to eq("fetched from example.com")
+ end
+ end
+
+ it "does not use a recursive glob that would pick up a nested SKILL.md" do
+ Dir.mktmpdir do |remote_root|
+ nested = File.join(remote_root, "example.com", "wrangler", "resources", "nested")
+ FileUtils.mkdir_p(nested)
+ File.write(File.join(nested, "SKILL.md"), <<~SKILL)
+ ---
+ name: nested-imposter
+ description: should not be discovered
+ ---
+ body
+ SKILL
+
+ catalog = described_class.new(load_paths: [described_class::GEM_SKILLS], remote_skills: remote_root)
+
+ expect(catalog.find("nested-imposter")).to be_nil
+ end
+ end
+
+ it "lets a user-level skill of the same name shadow a remote-fetched one" do
+ Dir.mktmpdir do |remote_root|
+ skill_dir = File.join(remote_root, "example.com", "tdd")
+ FileUtils.mkdir_p(skill_dir)
+ File.write(File.join(skill_dir, "SKILL.md"), <<~SKILL)
+ ---
+ name: tdd
+ description: remote version
+ ---
+ body
+ SKILL
+
+ catalog = described_class.new(load_paths: [described_class::GEM_SKILLS, fixtures], remote_skills: remote_root)
+
+ expect(catalog.find("tdd").description).to eq("Test-driven development. Use when the user wants tests written first.")
+ end
+ end
+
+ it "is not scanned when remote_skills is nil" do
+ catalog = described_class.new(load_paths: [described_class::GEM_SKILLS], remote_skills: nil)
+
+ expect { catalog.all }.not_to raise_error
+ end
+ end
+
describe "#find" do
it "returns nil for an unknown skill" do
catalog = described_class.new(load_paths: [fixtures])
spec/elelem/skills/fetcher_spec.rb
@@ -46,7 +46,7 @@ RSpec.describe Elelem::Skills::Fetcher do
names = fetcher.fetch("example.com")
expect(names).to eq(["example"])
- expect(File.read(File.join(install_dir, "example", "SKILL.md"))).to eq(skill_body)
+ expect(File.read(File.join(install_dir, "example.com", "example", "SKILL.md"))).to eq(skill_body)
end
it "adds https:// when the origin has no scheme" do
@@ -111,5 +111,48 @@ RSpec.describe Elelem::Skills::Fetcher do
expect { fetcher.fetch("example.com") }.to raise_error(Elelem::Skills::Fetcher::FetchError, /404/)
end
+
+ it "partitions by host so two origins publishing the same skill name don't clobber each other" do
+ other_body = "---\nname: example\ndescription: A different example skill, from another host.\n---\nOther body.\n"
+ other_digest = "sha256:#{Digest::SHA256.hexdigest(other_body)}"
+ other_index = {
+ "$schema" => Elelem::Skills::Fetcher::SCHEMA,
+ "skills" => [
+ { "name" => "example", "type" => "skill-md", "description" => "A different example skill.",
+ "url" => "/.well-known/agent-skills/example/SKILL.md", "digest" => other_digest }
+ ]
+ }.to_json
+
+ allow(http).to receive(:get_response)
+ .with(URI("https://example.com/.well-known/agent-skills/index.json"))
+ .and_return(ok(index))
+ allow(http).to receive(:get_response)
+ .with(URI("https://example.com/.well-known/agent-skills/example/SKILL.md"))
+ .and_return(ok(skill_body))
+ allow(http).to receive(:get_response)
+ .with(URI("https://other.example/.well-known/agent-skills/index.json"))
+ .and_return(ok(other_index))
+ allow(http).to receive(:get_response)
+ .with(URI("https://other.example/.well-known/agent-skills/example/SKILL.md"))
+ .and_return(ok(other_body))
+
+ fetcher.fetch("example.com")
+ fetcher.fetch("other.example")
+
+ expect(File.read(File.join(install_dir, "example.com", "example", "SKILL.md"))).to eq(skill_body)
+ expect(File.read(File.join(install_dir, "other.example", "example", "SKILL.md"))).to eq(other_body)
+ end
+
+ it "lowercases the host when using it as a directory name" do
+ index_url = URI("https://Example.COM/.well-known/agent-skills/index.json")
+ allow(http).to receive(:get_response).with(index_url).and_return(ok(index))
+ allow(http).to receive(:get_response)
+ .with(URI("https://Example.COM/.well-known/agent-skills/example/SKILL.md"))
+ .and_return(ok(skill_body))
+
+ fetcher.fetch("Example.COM")
+
+ expect(File.directory?(File.join(install_dir, "example.com"))).to be(true)
+ end
end
end
README.md
@@ -25,8 +25,16 @@ description: Test-driven development. Use when the user wants tests written firs
`.agents/backlog/`) with five phases (plan, design, build, review, verify)
as sibling files the model reads on demand. Used to live as five separate
ERB system prompts before the skills standard existed.
-2. `~/.agents/skills/*/SKILL.md` (user-level)
-3. `.agents/skills/*/SKILL.md` (project-level)
+2. `~/.agents/remote-skills/<host>/*/SKILL.md` -- skills installed via
+ `/skills fetch` (see below), partitioned by the index host they came
+ from so two publishers naming a skill the same thing don't clobber each
+ other's files on disk. Note this only prevents the file collision: if
+ two different hosts both publish a skill named `wrangler`, `Catalog`
+ still resolves `skill(name: "wrangler")` to whichever one was scanned
+ last (see precedence above) -- partitioning fixes storage, not naming
+ ambiguity across publishers.
+3. `~/.agents/skills/*/SKILL.md` (user-level)
+4. `.agents/skills/*/SKILL.md` (project-level)
for skills, and registers a `skill` toolbox tool whose *description* is the
skill index (name + frontmatter description for every discovered skill) --
@@ -52,14 +60,26 @@ Implements [cloudflare/agent-skills-discovery-rfc](https://github.com/cloudflare
3. For each `type: "skill-md"` entry, GETs the artifact and verifies its
SHA-256 against the index's `digest` before writing anything to disk;
a mismatch raises rather than installing unverified content
-4. Writes verified skills to `~/.agents/skills/<name>/SKILL.md`
+4. Writes verified skills to `~/.agents/remote-skills/<host>/<name>/SKILL.md`,
+ partitioned by the index's own host (not the artifact URL's host, which
+ the spec allows to point elsewhere, e.g. a CDN)
This is deliberately a separate, on-demand step from `Catalog` discovery,
not folded into it -- `Catalog#discover` runs synchronously on every
-agent build (including `/reload`), so it stays local-filesystem-only and
-fast. Fetching is the only place in this gem that touches the network.
-
-Run `/reload` after a fetch to pick up the newly installed skills.
+agent build, so it stays local-filesystem-only and fast. Fetching is the
+only place in this gem that touches the network.
+
+**Restart `elelem` after a fetch to pick up the newly installed skills --
+don't use `/reload`.** `Registry#reload` in elelem core clears all
+registered setups and toolbox tools, then re-`load`s files under
+`~/.agents/plugins`; but those files typically just `require
+"elelem/skills"`, and `require` is a no-op the second time (unlike
+`load`), so this gem's `config.setup(:skills)` block -- and therefore the
+`skill` tool itself -- never re-registers. Verified: after `/reload`,
+`agent.toolbox.tools.keys` is `[]`. A fresh `elelem` process picks up
+fetched skills correctly; this is a core gap in how `/reload` interacts
+with plugin files that use `require` instead of `load`, not something
+fixable from this gem.
**Not implemented**: `type: "archive"` entries (`.tar.gz`/`.zip` bundles
with `scripts/`/`references/`/`assets/`) are skipped with a warning --