Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
@@ -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>,
): string[] {
const references = new Set<string>();

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];
}
40 changes: 40 additions & 0 deletions packages/workspace-server/src/services/skills/skills.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
42 changes: 32 additions & 10 deletions packages/workspace-server/src/services/skills/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
parseSkillDependencies,
parseSkillFrontmatter,
} from "./parse-skill-frontmatter";
import { parseSkillReferences } from "./parse-skill-references";
import type {
BundleLocalSkillInput,
BundleLocalSkillOutput,
Expand Down Expand Up @@ -506,27 +507,43 @@ 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[],
): Promise<SkillBundleRef[]> {
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<string> = new Set(
allSkills.map((skill) => skill.name),
);

const seen = new Set<string>();
const resolved: SkillBundleRef[] = [];
Expand Down Expand Up @@ -558,15 +575,20 @@ export class SkillsService {
path.join(skillDir, "SKILL.md"),
"utf-8",
);
dependencyNames = parseSkillDependencies(manifest);
dependencyNames = [
...new Set([
...parseSkillDependencies(manifest),
...parseSkillReferences(manifest, knownSkillNames),
Comment thread
tatoalo marked this conversation as resolved.
]),
];
} 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.
continue;
}

for (const dependencyName of dependencyNames) {
const dependencyRef = findUploadableByName(dependencyName);
const dependencyRef = findUploadableByName(dependencyName, ref);
if (
dependencyRef &&
!seen.has(`${dependencyRef.source}:${dependencyRef.path}`)
Expand Down
Loading