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
58 changes: 57 additions & 1 deletion packages/dotagents-lib/src/skills/resolver.integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
import { join } from "node:path";
import { tmpdir } from "node:os";
Expand Down Expand Up @@ -67,6 +67,7 @@ description: Code review skill
});

afterEach(async () => {
vi.unstubAllEnvs();
await rm(tmpDir, { recursive: true });
});

Expand Down Expand Up @@ -134,6 +135,61 @@ description: A local skill
}
});

it("prefers an explicit ref over an inline ref for single and wildcard resolution", async () => {
await exec("git", ["branch", "inline-ref"], { cwd: repoDir });
await exec("git", ["checkout", "-b", "explicit-ref"], { cwd: repoDir });
await mkdir(join(repoDir, "explicit-only"), { recursive: true });
await writeFile(
join(repoDir, "explicit-only", "SKILL.md"),
`---
name: explicit-only
description: Only available from the explicit ref
---
`,
);
await exec("git", ["add", "."], { cwd: repoDir });
await exec("git", ["commit", "-m", "add explicit-only skill"], {
cwd: repoDir,
});

vi.stubEnv("GIT_CONFIG_COUNT", "1");
vi.stubEnv(
"GIT_CONFIG_KEY_0",
`url.file://${repoDir}.insteadOf`,
);
vi.stubEnv(
"GIT_CONFIG_VALUE_0",
"https://github.com/example/skills",
);

const source = "https://github.com/example/skills@inline-ref";
const [single, wildcard] = await Promise.all([
resolveSkill(
"explicit-only",
{ source, ref: "explicit-ref" },
{ stateDir: join(stateDir, "single") },
),
resolveWildcardSkills(
{ source, ref: "explicit-ref" },
{ stateDir: join(stateDir, "wildcard") },
),
]);

expect(single).toMatchObject({
type: "git",
resolvedRef: "explicit-ref",
resolvedPath: "explicit-only",
});
expect(wildcard).toContainEqual({
name: "explicit-only",
resolved: expect.objectContaining({
type: "git",
resolvedRef: "explicit-ref",
resolvedPath: "explicit-only",
}),
});
});

it("throws ResolveError when skill not found in repo", async () => {
await expect(
resolveSkill(
Expand Down
182 changes: 89 additions & 93 deletions packages/dotagents-lib/src/skills/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,44 +325,93 @@ export interface ResolveOpts {
trust?: TrustPolicy;
}

export async function resolveSkill(
skillName: string,
dep: { source: string; ref?: string; path?: string },
type AcquiredSkillSource =
| { type: "local"; skillDir: string }
| { type: "well-known"; cacheDir: string | null; resolvedUrl: string }
| {
type: "git";
repoDir: string;
resolvedUrl: string;
resolvedRef?: string;
commit: string;
};

async function acquireSkillSource(
dep: { source: string; ref?: string },
opts: ResolveOpts,
): Promise<ResolvedSkill> {
): Promise<AcquiredSkillSource> {
const sourceForResolve = applyDefaultRepositorySource(
dep.source,
opts.defaultRepositorySource,
);
// Validate the EXPANDED source so that owner/repo shorthand under a non-default
// host (e.g. defaultRepositorySource: "gitlab") is checked against the actual
// clone URL, not the shorthand's implicit github.com mapping.
// Trust the expanded source before any local or network acquisition.
if (opts.trust) {validateTrustedSource(sourceForResolve, opts.trust);}

const parsed = parseSource(sourceForResolve);

if (parsed.type === "local") {
const projectRoot = opts.projectRoot || process.cwd();
const skillDir = await resolveLocalSource(projectRoot, parsed.path!);
return { type: "local", source: dep.source, skillDir };
return { type: "local", skillDir };
}

if (parsed.type === "well-known") {
const baseUrl = parsed.url!;
const resolvedUrl = parsed.url!;
const cached = await ensureWellKnownCached({
stateDir: opts.stateDir,
url: baseUrl,
cacheKey: wellKnownCacheKey(baseUrl),
url: resolvedUrl,
cacheKey: wellKnownCacheKey(resolvedUrl),
ttlMs: opts.ttlMs,
});
return {
type: "well-known",
cacheDir: cached?.cacheDir ?? null,
resolvedUrl,
};
}

const url = parsed.url!;
const resolvedUrl = parsed.cloneUrl ?? url;
const resolvedRef = dep.ref ?? parsed.ref;
const cacheKey = parsed.type === "github"
? `${parsed.owner}/${parsed.repo}`
: sanitizeCacheKey(url);
const excluded = isSourceExcluded(dep.source, opts.minimumReleaseAgeExclude);
const cached = await ensureCached({
stateDir: opts.stateDir,
url: resolvedUrl,
cacheKey,
ref: resolvedRef,
minimumReleaseAge: excluded ? undefined : opts.minimumReleaseAge,
});

return {
type: "git",
repoDir: cached.repoDir,
resolvedUrl,
resolvedRef,
commit: cached.commit,
};
}

export async function resolveSkill(
skillName: string,
dep: { source: string; ref?: string; path?: string },
opts: ResolveOpts,
): Promise<ResolvedSkill> {
const acquired = await acquireSkillSource(dep, opts);

if (!cached) {
if (acquired.type === "local") {
return { type: "local", source: dep.source, skillDir: acquired.skillDir };
}

if (acquired.type === "well-known") {
if (!acquired.cacheDir) {
throw new ResolveError(
`No skills found at ${baseUrl}. If this is a git repository, use the git: prefix: git:${baseUrl}`,
`No skills found at ${acquired.resolvedUrl}. If this is a git repository, use the git: prefix: git:${acquired.resolvedUrl}`,
);
}

const discovered = await discoverSkill(cached.cacheDir, skillName, { scanDirs: opts.scanDirs });
const discovered = await discoverSkill(acquired.cacheDir, skillName, { scanDirs: opts.scanDirs });
if (!discovered) {
throw new ResolveError(
`Skill "${skillName}" not found in ${dep.source}. ` +
Expand All @@ -373,37 +422,17 @@ export async function resolveSkill(
return {
type: "well-known",
source: dep.source,
resolvedUrl: baseUrl,
skillDir: join(cached.cacheDir, discovered.path),
resolvedUrl: acquired.resolvedUrl,
skillDir: join(acquired.cacheDir, discovered.path),
};
}

// Git source (GitHub or generic git)
const url = parsed.url!;
const cloneUrl = parsed.cloneUrl ?? url;
const ref = dep.ref ?? parsed.ref;
const cacheKey =
parsed.type === "github"
? `${parsed.owner}/${parsed.repo}`
: sanitizeCacheKey(url);

const excluded = isSourceExcluded(dep.source, opts.minimumReleaseAgeExclude);
const cached = await ensureCached({
stateDir: opts.stateDir,
url: cloneUrl,
cacheKey,
ref,
minimumReleaseAge: excluded ? undefined : opts.minimumReleaseAge,
});

// Discover the skill within the repo
let discovered: DiscoveredSkill | null;
if (dep.path) {
// Explicit path override — load directly
const meta = await loadSkillMd(join(cached.repoDir, dep.path, "SKILL.md"));
const meta = await loadSkillMd(join(acquired.repoDir, dep.path, "SKILL.md"));
discovered = { path: dep.path, meta };
} else {
discovered = await discoverSkill(cached.repoDir, skillName, { scanDirs: opts.scanDirs });
discovered = await discoverSkill(acquired.repoDir, skillName, { scanDirs: opts.scanDirs });
}

if (!discovered) {
Expand All @@ -416,11 +445,11 @@ export async function resolveSkill(
return {
type: "git",
source: dep.source,
resolvedUrl: cloneUrl,
resolvedUrl: acquired.resolvedUrl,
resolvedPath: discovered.path,
resolvedRef: ref,
commit: cached.commit,
skillDir: join(cached.repoDir, discovered.path),
resolvedRef: acquired.resolvedRef,
commit: acquired.commit,
skillDir: join(acquired.repoDir, discovered.path),
};
}

Expand All @@ -440,21 +469,11 @@ export async function resolveWildcardSkills(
dep: WildcardDependencyInput,
opts: ResolveOpts,
): Promise<NamedResolvedSkill[]> {
const sourceForResolve = applyDefaultRepositorySource(
dep.source,
opts.defaultRepositorySource,
);
// See note on resolveSkill: validate the expanded source so shorthand under a
// non-default host can't bypass the policy.
if (opts.trust) {validateTrustedSource(sourceForResolve, opts.trust);}

const parsed = parseSource(sourceForResolve);
const acquired = await acquireSkillSource(dep, opts);
const excludeSet = new Set(dep.exclude);

if (parsed.type === "local") {
const projectRoot = opts.projectRoot || process.cwd();
const skillDir = await resolveLocalSource(projectRoot, parsed.path!);
const discovered = await discoverAllSkills(skillDir, { scanDirs: opts.scanDirs });
if (acquired.type === "local") {
const discovered = await discoverAllSkills(acquired.skillDir, { scanDirs: opts.scanDirs });
return discovered
.filter(
(d) =>
Expand All @@ -465,25 +484,20 @@ export async function resolveWildcardSkills(
resolved: {
type: "local" as const,
source: dep.source,
skillDir: join(skillDir, d.path),
skillDir: join(acquired.skillDir, d.path),
},
}));
}

if (parsed.type === "well-known") {
const baseUrl = parsed.url!;
const cached = await ensureWellKnownCached({
stateDir: opts.stateDir,
url: baseUrl,
cacheKey: wellKnownCacheKey(baseUrl),
ttlMs: opts.ttlMs,
});

if (!cached) {
if (acquired.type === "well-known") {
if (!acquired.cacheDir) {
return [];
}

const discovered = await discoverAllSkills(cached.cacheDir, { scanDirs: opts.scanDirs });
const cacheDir = acquired.cacheDir;
const discovered = await discoverAllSkills(cacheDir, {
scanDirs: opts.scanDirs,
});
return discovered
.filter(
(d) => !excludeSet.has(d.meta.name) && VALID_SKILL_NAME.test(d.meta.name),
Expand All @@ -493,31 +507,13 @@ export async function resolveWildcardSkills(
resolved: {
type: "well-known" as const,
source: dep.source,
resolvedUrl: baseUrl,
skillDir: join(cached.cacheDir, d.path),
resolvedUrl: acquired.resolvedUrl,
skillDir: join(cacheDir, d.path),
},
}));
}

// Git source
const url = parsed.url!;
const cloneUrl = parsed.cloneUrl ?? url;
const ref = dep.ref ?? parsed.ref;
const cacheKey =
parsed.type === "github"
? `${parsed.owner}/${parsed.repo}`
: sanitizeCacheKey(url);

const excluded = isSourceExcluded(dep.source, opts.minimumReleaseAgeExclude);
const cached = await ensureCached({
stateDir: opts.stateDir,
url: cloneUrl,
cacheKey,
ref,
minimumReleaseAge: excluded ? undefined : opts.minimumReleaseAge,
});

const discovered = await discoverAllSkills(cached.repoDir, { scanDirs: opts.scanDirs });
const discovered = await discoverAllSkills(acquired.repoDir, { scanDirs: opts.scanDirs });

return discovered
.filter(
Expand All @@ -528,11 +524,11 @@ export async function resolveWildcardSkills(
resolved: {
type: "git" as const,
source: dep.source,
resolvedUrl: cloneUrl,
resolvedUrl: acquired.resolvedUrl,
resolvedPath: d.path,
resolvedRef: ref,
commit: cached.commit,
skillDir: join(cached.repoDir, d.path),
resolvedRef: acquired.resolvedRef,
commit: acquired.commit,
skillDir: join(acquired.repoDir, d.path),
},
}));
}
Expand Down
26 changes: 26 additions & 0 deletions packages/dotagents-lib/src/skills/resolver.wellknown.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { describe, expect, it, vi } from "vitest";
import { ResolveError, resolveSkill, resolveWildcardSkills } from "./resolver.js";

vi.mock("../sources/wellknown.js", () => ({
ensureWellKnownCached: vi.fn().mockResolvedValue(null),
}));

const STATE_DIR = "/tmp/dotagents-well-known-test-cache";
const SOURCE = "https://skills.example.com";

describe("missing well-known index", () => {
it("rejects single-skill resolution", async () => {
await expect(
resolveSkill("review", { source: SOURCE }, { stateDir: STATE_DIR }),
).rejects.toThrow(ResolveError);
});

it("returns no wildcard skills", async () => {
await expect(
resolveWildcardSkills(
{ source: SOURCE, exclude: [] },
{ stateDir: STATE_DIR },
),
).resolves.toEqual([]);
});
});
Loading