From 5db545b245f118e9a1c1d4a1476a036c3fa93e5d Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Mon, 13 Jul 2026 17:47:14 +0100 Subject: [PATCH] feat(skills): auto-bundle skills referenced in a skill's prose for cloud runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dependency auto-bundling (#3057) only follows the frontmatter dependencies: list, but most skills point at each other in prose — "run /other-skill next" or [[other-skill]]. A referenced skill that was never declared stayed on the user's machine, so the sandbox agent couldn't load it when the parent skill invoked it. resolveSkillBundleDependencies now also scans each SKILL.md body for /skill-name and [[skill-name]] tokens. Matches are filtered against the known local skill names, resolution prefers a skill beside the referencing one (then same source) so a repo skill referencing /helper gets its sibling, and uppercase frontmatter names are recognized. The transitive expansion, bundled-skill skip, cycle guard, and 50-skill ceiling apply unchanged. --- .../skills/parse-skill-references.test.ts | 36 ++++++++++++++++ .../services/skills/parse-skill-references.ts | 33 +++++++++++++++ .../src/services/skills/skills.test.ts | 40 ++++++++++++++++++ .../src/services/skills/skills.ts | 42 ++++++++++++++----- 4 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 packages/workspace-server/src/services/skills/parse-skill-references.test.ts create mode 100644 packages/workspace-server/src/services/skills/parse-skill-references.ts diff --git a/packages/workspace-server/src/services/skills/parse-skill-references.test.ts b/packages/workspace-server/src/services/skills/parse-skill-references.test.ts new file mode 100644 index 0000000000..ff90409bae --- /dev/null +++ b/packages/workspace-server/src/services/skills/parse-skill-references.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { parseSkillReferences } from "./parse-skill-references"; + +const KNOWN = new Set(["rs-review", "dep-skill", "usr", "foo", "My-Helper"]); + +describe("parseSkillReferences", () => { + it.each([ + ["a bare slash reference", "Run /rs-review on the diff.", ["rs-review"]], + ["a reference at line start", "/rs-review\nthen stop", ["rs-review"]], + [ + "backticked and quoted references", + 'Use `/rs-review` or "/dep-skill" here.', + ["rs-review", "dep-skill"], + ], + ["a parenthesized reference", "(see /dep-skill)", ["dep-skill"]], + ["a wiki-style link", "Details in [[dep-skill]].", ["dep-skill"]], + [ + "mixed reference styles, deduplicated", + "Run /rs-review, then [[rs-review]] again and /dep-skill.", + ["rs-review", "dep-skill"], + ], + ["a sentence-final reference", "First run /dep-skill.", ["dep-skill"]], + [ + "an uppercase frontmatter name", + "Use /My-Helper then [[My-Helper]].", + ["My-Helper"], + ], + ["an unknown skill name", "Run /not-a-skill now.", []], + ["a URL path segment", "See https://example.com/foo for docs.", []], + ["a file path segment", "Look in /usr/bin and /foo/bar.", []], + ["a mid-word slash", "either/foo works", []], + ["an empty body", "", []], + ])("handles %s", (_label, content, expected) => { + expect(parseSkillReferences(content, KNOWN)).toEqual(expected); + }); +}); diff --git a/packages/workspace-server/src/services/skills/parse-skill-references.ts b/packages/workspace-server/src/services/skills/parse-skill-references.ts new file mode 100644 index 0000000000..faa838e15c --- /dev/null +++ b/packages/workspace-server/src/services/skills/parse-skill-references.ts @@ -0,0 +1,33 @@ +const SLASH_REFERENCE_REGEX = + /(?<=^|[\s(`"'[])\/([A-Za-z0-9][A-Za-z0-9._-]*)(?![A-Za-z0-9._/-])/gm; +const WIKI_LINK_REGEX = /\[\[([A-Za-z0-9][A-Za-z0-9._-]*)\]\]/g; + +/** + * Finds `/skill-name` and `[[skill-name]]` references in a SKILL.md body. + * Only names in `knownNames` are returned, so paths (`/usr/bin`), URL + * segments, and unrelated slash-words can't match. A slash reference must + * span its whole path segment — `/foo/bar` never matches a skill named `foo`. + */ +export function parseSkillReferences( + content: string, + knownNames: ReadonlySet, +): string[] { + const references = new Set(); + + for (const regex of [SLASH_REFERENCE_REGEX, WIKI_LINK_REGEX]) { + for (const match of content.matchAll(regex)) { + const name = match[1]; + if (knownNames.has(name)) { + references.add(name); + continue; + } + // dots are valid name chars, so a sentence-final "/dep-skill." captures the period + const withoutTrailingDots = name.replace(/\.+$/, ""); + if (withoutTrailingDots !== name && knownNames.has(withoutTrailingDots)) { + references.add(withoutTrailingDots); + } + } + } + + return [...references]; +} diff --git a/packages/workspace-server/src/services/skills/skills.test.ts b/packages/workspace-server/src/services/skills/skills.test.ts index ae2bb42a05..dc06bf1b91 100644 --- a/packages/workspace-server/src/services/skills/skills.test.ts +++ b/packages/workspace-server/src/services/skills/skills.test.ts @@ -625,6 +625,46 @@ describe("resolveSkillBundleDependencies", () => { .join("\n")}\n---\nbody`; } + it("expands prose references (/name and [[name]]) into dependencies", async () => { + const primary = await createSkill( + repoSkillsDir, + "prose-parent", + `---\nname: prose-parent\ndescription: parent\n---\nRun /prose-dep first, then see [[prose-wiki-dep]]. Ignore /usr/bin and /unknown-skill.`, + ); + const slashDep = await createSkill(repoSkillsDir, "prose-dep"); + const wikiDep = await createSkill(repoSkillsDir, "prose-wiki-dep"); + const service = makeService(); + + const resolved = await service.resolveSkillBundleDependencies([ + ref("prose-parent", primary), + ]); + + expect(resolved.map((r) => r.name)).toEqual([ + "prose-parent", + "prose-dep", + "prose-wiki-dep", + ]); + expect(resolved.map((r) => r.path)).toEqual([primary, slashDep, wikiDep]); + }); + + it("prefers a dependency beside the referencing skill over a same-named skill elsewhere", async () => { + const primary = await createSkill( + repoSkillsDir, + "scoped-parent", + withDeps("scoped-parent", ["helper"]), + ); + const repoHelper = await createSkill(repoSkillsDir, "helper"); + await mkdir(userSkillsHome.dir, { recursive: true }); + await createSkill(userSkillsHome.dir, "helper"); + const service = makeService(); + + const resolved = await service.resolveSkillBundleDependencies([ + ref("scoped-parent", primary), + ]); + + expect(resolved.map((r) => r.path)).toEqual([primary, repoHelper]); + }); + it("expands a tagged skill to include its transitive dependencies", async () => { const primary = await createSkill( repoSkillsDir, diff --git a/packages/workspace-server/src/services/skills/skills.ts b/packages/workspace-server/src/services/skills/skills.ts index 9f5db65f0a..d5de57be0c 100644 --- a/packages/workspace-server/src/services/skills/skills.ts +++ b/packages/workspace-server/src/services/skills/skills.ts @@ -16,6 +16,7 @@ import { parseSkillDependencies, parseSkillFrontmatter, } from "./parse-skill-frontmatter"; +import { parseSkillReferences } from "./parse-skill-references"; import type { BundleLocalSkillInput, BundleLocalSkillOutput, @@ -506,12 +507,12 @@ export class SkillsService { } /** - * Expand a set of tagged skill refs to include their transitively-declared - * dependency skills (SKILL.md `dependencies:`), so a skill that needs another - * (e.g. `/rs-self-review` → `rs-adversarial-review`) pulls its dependency into - * the same cloud run instead of the user having to tag every one by hand. - * Only uploadable local skills are returned; a dependency that resolves to a - * built-in (`bundled`) skill is already present in the sandbox and is skipped. + * Expand a set of tagged skill refs to include their transitive dependency + * skills, so a skill that needs another pulls it into the same cloud run. + * A dependency is either declared (SKILL.md frontmatter `dependencies:`) + * or referenced in the SKILL.md body as `/skill-name` or `[[skill-name]]`. + * Only uploadable local skills are returned; a dependency that resolves to + * a built-in (`bundled`) skill is already in the sandbox and is skipped. */ async resolveSkillBundleDependencies( refs: SkillBundleRef[], @@ -519,14 +520,30 @@ export class SkillsService { if (refs.length === 0) return []; const allSkills = await this.listSkills(); - const findUploadableByName = (name: string): SkillBundleRef | null => { - const match = allSkills.find( + // Prefer a dependency beside the referencing skill (same root), then one + // from the same source, so a repo skill referencing /helper gets its + // sibling rather than a same-named skill from another source. + const findUploadableByName = ( + name: string, + referencedFrom: SkillBundleRef, + ): SkillBundleRef | null => { + const candidates = allSkills.filter( (skill) => skill.name === name && isUploadableSkillSource(skill.source), ); + const match = + candidates.find( + (skill) => + path.dirname(skill.path) === path.dirname(referencedFrom.path), + ) ?? + candidates.find((skill) => skill.source === referencedFrom.source) ?? + candidates[0]; return match && isUploadableSkillSource(match.source) ? { name: match.name, source: match.source, path: match.path } : null; }; + const knownSkillNames: ReadonlySet = new Set( + allSkills.map((skill) => skill.name), + ); const seen = new Set(); const resolved: SkillBundleRef[] = []; @@ -558,7 +575,12 @@ export class SkillsService { path.join(skillDir, "SKILL.md"), "utf-8", ); - dependencyNames = parseSkillDependencies(manifest); + dependencyNames = [ + ...new Set([ + ...parseSkillDependencies(manifest), + ...parseSkillReferences(manifest, knownSkillNames), + ]), + ]; } catch { // A ref we can't read (missing/renamed skill) still uploads on its own; // just skip its dependency expansion rather than failing the whole run. @@ -566,7 +588,7 @@ export class SkillsService { } for (const dependencyName of dependencyNames) { - const dependencyRef = findUploadableByName(dependencyName); + const dependencyRef = findUploadableByName(dependencyName, ref); if ( dependencyRef && !seen.has(`${dependencyRef.source}:${dependencyRef.path}`)