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
31 changes: 20 additions & 11 deletions packages/cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,13 +536,12 @@ interface CliInstallContext {

function buildInstallContext(
selection: PersonaSelection,
options: { installRoot?: string } = {}
options: { installRoot?: string; repoRoot?: string } = {}
): CliInstallContext {
const plan = materializeSkills(
selection.skills,
selection.runtime.harness,
options.installRoot !== undefined ? { installRoot: options.installRoot } : {}
);
const plan = materializeSkills(selection.skills, selection.runtime.harness, {
...(options.installRoot !== undefined ? { installRoot: options.installRoot } : {}),
...(options.repoRoot !== undefined ? { repoRoot: options.repoRoot } : {})
});
const { installCommand, installCommandString } = buildInstallArtifacts(plan);
const { cleanupCommand, cleanupCommandString } = buildCleanupArtifacts(plan);
return {
Expand Down Expand Up @@ -1038,7 +1037,12 @@ function runDryRun(selection: PersonaSelection): number {
process.stderr.write(`✓ harness spec: ${spec.bin} (${spec.args.length} args)\n`);

// Check 3: skill installs.
const plan = materializeSkills(effectiveSelection.skills, runtime.harness);
// Dry-run runs each install inside a fresh tempDir (see `cwd: tempDir` on
// the spawnSync below). Pass repoRoot=process.cwd() so `local`-kind skills
// resolve their relative source paths against the real repo, not the tmp.
const plan = materializeSkills(effectiveSelection.skills, runtime.harness, {
repoRoot: process.cwd()
});
if (plan.installs.length === 0) {
process.stderr.write('✓ skills: (none declared)\n');
process.stderr.write('✓ dry-run ok\n');
Expand Down Expand Up @@ -1192,10 +1196,15 @@ async function runInteractive(
sessionRoot && runtime.harness === 'claude'
? sessionInstallRoot(sessionRoot)
: undefined;
const install = buildInstallContext(
effectiveSelection,
installRoot !== undefined ? { installRoot } : {}
);
// `repoRoot` lets the local skill provider (kind: 'local') resolve
// relative source paths like `.agentworkforce/workforce/skills/foo.md` to
// absolute paths before the install command runs. Critical for claude's
// installRoot mode, where the install runs `cd <installRoot> && cp ...`
// and a relative source would otherwise resolve against the wrong dir.
const install = buildInstallContext(effectiveSelection, {
...(installRoot !== undefined ? { installRoot } : {}),
repoRoot: process.cwd()
});
process.stderr.write(`→ ${personaId} [${tier}] via ${runtime.harness} (${runtime.model})\n`);

const startLaunchMetadataForLaunch = (cwd = process.cwd()) =>
Expand Down
2 changes: 1 addition & 1 deletion packages/persona-kit/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export const PERMISSION_MODES = [
'plan'
] as const;

export const SKILL_SOURCE_KINDS = ['prpm', 'skill.sh'] as const;
export const SKILL_SOURCE_KINDS = ['prpm', 'skill.sh', 'local'] as const;

export const HARNESS_SKILL_TARGETS: Record<Harness, HarnessSkillTarget> = {
claude: { asFlag: 'claude', dir: '.claude/skills' },
Expand Down
127 changes: 127 additions & 0 deletions packages/persona-kit/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,133 @@ test('materializeSkills handles personas with no skills', () => {
assert.equal(plan.installs.length, 0);
});

const localSkill = {
id: 'local/essay-authoring',
source: '.agentworkforce/workforce/skills/essay-authoring.md',
description: 'repo-local SKILL.md'
};

test('materializeSkills emits a mkdir+cp install for a local .md source', () => {
const plan = materializeSkills([localSkill], 'claude');

assert.equal(plan.installs.length, 1);
const [install] = plan.installs;
assert.equal(install.sourceKind, 'local');
assert.equal(install.packageRef, '.agentworkforce/workforce/skills/essay-authoring.md');
assert.equal(install.installedDir, '.claude/skills/essay-authoring');
assert.equal(install.installedManifest, '.claude/skills/essay-authoring/SKILL.md');
assert.deepEqual([...install.cleanupPaths], ['.claude/skills/essay-authoring']);
assert.deepEqual([...install.installCommand], [
'sh',
'-c',
'mkdir -p .claude/skills/essay-authoring && cp .agentworkforce/workforce/skills/essay-authoring.md .claude/skills/essay-authoring/SKILL.md'
]);
});

test('local sources accept ./-prefixed and absolute paths and strip the .md extension for the installed name', () => {
const plan = materializeSkills(
[
{ id: 'dot-slash', source: './skills/dot-slash.md', description: 'leading ./' },
{ id: 'abs', source: '/abs/path/abs-skill.md', description: 'absolute path' }
],
'codex'
);
assert.equal(plan.installs[0].installedDir, '.agents/skills/dot-slash');
assert.equal(plan.installs[1].installedDir, '.agents/skills/abs-skill');
assert.match(plan.installs[1].installCommand[2] ?? '', / \/abs\/path\/abs-skill\.md /);
});

test('local sources using SKILL.md adopt the parent dir name as the installed name', () => {
const plan = materializeSkills(
[
{
id: 'my-skill',
source: '.agentworkforce/workforce/skills/my-skill/SKILL.md',
description: 'directory-shaped local skill'
}
],
'claude'
);
const [install] = plan.installs;
assert.equal(install.installedDir, '.claude/skills/my-skill');
});

test('local source with repoRoot absoluteifies the cp source path', () => {
const plan = materializeSkills([localSkill], 'claude', {
repoRoot: '/home/user/project'
});
const [install] = plan.installs;
assert.equal(install.sourceKind, 'local');
assert.equal(plan.repoRoot, '/home/user/project');
assert.match(
install.installCommand[2] ?? '',
/cp \/home\/user\/project\/\.agentworkforce\/workforce\/skills\/essay-authoring\.md /
);
});

test('local source absolute path is left alone even with repoRoot', () => {
const plan = materializeSkills(
[{ id: 'abs', source: '/abs/path/foo.md', description: 'absolute' }],
'claude',
{ repoRoot: '/home/user/project' }
);
const script = plan.installs[0].installCommand[2] ?? '';
assert.match(script, / \/abs\/path\/foo\.md /);
assert.ok(!script.includes('/home/user/project/abs/path'));
});

test('local source survives installRoot session mode by embedding the absolute repo path', () => {
const installRoot = '/tmp/agent-workforce/sessions/local-test/claude/plugin';
const plan = materializeSkills([localSkill], 'claude', {
installRoot,
repoRoot: '/home/user/project'
});
const [install] = plan.installs;
assert.equal(install.installCommand[0], 'sh');
assert.equal(install.installCommand[1], '-c');
const script = install.installCommand[2] ?? '';
// Outer wrapper: cd <installRoot> && <inner>; inner is the sh -c mkdir+cp.
assert.match(script, /^cd /);
assert.match(script, /\/local-test\/claude\/plugin/);
assert.match(script, /\/home\/user\/project\/\.agentworkforce\/workforce\/skills\/essay-authoring\.md/);
// Per-skill cleanupPaths stay empty in session mode (cleaned via stage dir).
assert.deepEqual([...install.cleanupPaths], []);
});

test('local source does NOT shadow prpm bare references like coreyhaines31/marketingskills', () => {
// The local provider is registered first; it must only claim sources that
// look like local .md paths. A bare prpm <scope>/<name> ref must still go
// to the prpm provider.
const plan = materializeSkills(
[
{
id: 'bare-prpm',
source: 'coreyhaines31/marketingskills',
description: 'prpm bare ref'
}
],
'claude'
);
assert.equal(plan.installs[0].sourceKind, 'prpm');
});

test('local source rejects paths without .md suffix', () => {
assert.throws(
() =>
materializeSkills(
[
{
id: 'dir',
source: './skills/my-skill',
description: 'no .md suffix'
}
],
'claude'
),
/Unsupported skill source/
);
});

test('resolveSidecar: tier path override drops top-level inlined content for the same channel', () => {
const spec = syntheticSpec({
claudeMdContent: '# top-level inlined\n',
Expand Down
115 changes: 108 additions & 7 deletions packages/persona-kit/src/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,25 @@ interface ResolvedSkillSource {
installedName: string;
}

/**
* Subset of {@link SkillMaterializationOptions} that providers care about when
* building install commands. Forwarded by `materializeSkills` and the
* `buildInstallArtifacts` chain. Optional so providers can ignore it.
*/
interface BuildInstallContext {
repoRoot?: string;
}

interface SkillProvider {
readonly kind: SkillSourceKind;
/** Parse a persona `source` string; return null if this provider does not claim it. */
parse(source: string): ResolvedSkillSource | null;
/** Build the argv-style install command for `materializeSkills`. */
buildInstallCommand(ref: ResolvedSkillSource, harness: Harness): readonly string[];
buildInstallCommand(
ref: ResolvedSkillSource,
harness: Harness,
context?: BuildInstallContext
): readonly string[];
/**
* Ephemeral paths the installer scatters for this skill under `harness` that are
* safe to remove once the persona has finished reading them. Does not include
Expand Down Expand Up @@ -170,7 +183,86 @@ const skillShProvider: SkillProvider = {
}
};

const SKILL_PROVIDERS: readonly SkillProvider[] = Object.freeze([prpmProvider, skillShProvider]);
// Local source forms:
// - A repo-relative or absolute path to a single `.md` file that will be
// installed as `<harness-skill-dir>/<name>/SKILL.md`.
// Examples:
// - `.agentworkforce/workforce/skills/essay-authoring.md`
// - `./skills/my-skill.md`
// - `/abs/path/to/SKILL.md`
//
// The provider intentionally rejects URLs and prpm `<scope>/<name>` shorthand
// (no `.md` suffix) so it never claims sources the other providers should
// handle. It is registered FIRST so its narrow `.md`-suffix test gets first
// refusal on path-shaped strings before prpm's bare-ref regex sees them.
const LOCAL_MD_RE = /\.md$/i;
const URL_PREFIX_RE = /^[a-z][a-z0-9+.-]*:\/\//i;

function localInstalledName(source: string): string | null {
const lastSlash = source.lastIndexOf('/');
const basename = lastSlash >= 0 ? source.slice(lastSlash + 1) : source;
const stem = basename.replace(/\.md$/i, '');
// A SKILL.md inside a skill dir is conventional; in that case the parent
// directory name is the meaningful identifier. Strip a trailing `/SKILL`
// stem and use the segment above it instead.
if (stem === 'SKILL' && lastSlash > 0) {
const parentSlice = source.slice(0, lastSlash);
const parentSlash = parentSlice.lastIndexOf('/');
const parent = parentSlash >= 0 ? parentSlice.slice(parentSlash + 1) : parentSlice;
return toSafeSkillName(parent);
}
return toSafeSkillName(stem);
}

const localProvider: SkillProvider = {
kind: 'local',
parse(source) {
if (URL_PREFIX_RE.test(source)) return null;
if (!LOCAL_MD_RE.test(source)) return null;
const installedName = localInstalledName(source);
if (!installedName) return null;
return {
kind: 'local',
// packageRef preserves the original source string verbatim so the
// command builder can resolve it against `repoRoot` if supplied.
packageRef: source,
installedName
};
},
buildInstallCommand(ref, harness, context) {
const target = HARNESS_SKILL_TARGETS[harness];
const destDir = `${target.dir}/${ref.installedName}`;
const source = resolveLocalSource(ref.packageRef, context?.repoRoot);
// A single shell command so the mkdir + cp pair is atomic from the
// caller's perspective (one spawn, one exit code) and survives a
// `cd <installRoot> && …` wrapper in session mode — the destination
// stays harness-relative, the source is absolute when `repoRoot` is set.
const script = `mkdir -p ${shellEscape(destDir)} && cp ${shellEscape(source)} ${shellEscape(`${destDir}/SKILL.md`)}`;
return Object.freeze(['sh', '-c', script]) as readonly string[];
},
cleanupPaths(ref, harness) {
const target = HARNESS_SKILL_TARGETS[harness];
return Object.freeze([`${target.dir}/${ref.installedName}`]) as readonly string[];
}
};

function resolveLocalSource(source: string, repoRoot: string | undefined): string {
// Absolute paths and `file://` URLs (already stripped above for the URL
// check, but kept defensive here) stand on their own. Relative paths are
// joined to `repoRoot` when supplied so session-mode `cd <installRoot>`
// doesn't change their meaning.
if (source.startsWith('/')) return source;
if (!repoRoot) return source;
const trimmedRoot = repoRoot.endsWith('/') ? repoRoot.slice(0, -1) : repoRoot;
const trimmedSource = source.startsWith('./') ? source.slice(2) : source;
return `${trimmedRoot}/${trimmedSource}`;
}

const SKILL_PROVIDERS: readonly SkillProvider[] = Object.freeze([
localProvider,
prpmProvider,
skillShProvider
]);

export function resolveSkillSource(source: string): ResolvedSkillSource {
for (const provider of SKILL_PROVIDERS) {
Expand All @@ -184,7 +276,8 @@ export function resolveSkillSource(source: string): ResolvedSkillSource {
`Supported forms: prpm.dev package URL (https://prpm.dev/packages/<scope>/<name>), ` +
`bare "<scope>/<name>" prpm reference, ` +
`skill.sh github URL with skill fragment (https://github.com/<org>/<repo>#<skill>), ` +
`or GitHub tree URL to a skill directory (https://github.com/<org>/<repo>/tree/<ref>/<skill>).`
`GitHub tree URL to a skill directory (https://github.com/<org>/<repo>/tree/<ref>/<skill>), ` +
`or a local repo-relative path to a SKILL markdown file (e.g. ./skills/my-skill.md, .agentworkforce/workforce/skills/my-skill.md).`
);
}

Expand Down Expand Up @@ -213,18 +306,21 @@ export function materializeSkills(
if (!target) {
throw new Error(`No skill install target configured for harness: ${harness}`);
}
const { installRoot } = options;
const { installRoot, repoRoot } = options;
if (installRoot !== undefined && harness !== 'claude') {
throw new Error(
`installRoot is only supported for the claude harness (got: ${harness}). ` +
`codex and opencode still install into the harness's conventional repo-relative directory.`
);
}

const providerContext: BuildInstallContext | undefined =
repoRoot !== undefined ? { repoRoot } : undefined;

const installs = skills.map((skill): SkillInstall => {
const resolved = resolveSkillSource(skill.source);
const provider = providerFor(resolved.kind);
const baseCommand = provider.buildInstallCommand(resolved, harness);
const baseCommand = provider.buildInstallCommand(resolved, harness, providerContext);
// In session-install-root mode, the install runs `cd <installRoot> && <prpm>`
// so that prpm's harness-relative dirs (`.claude/skills/<name>`) land
// inside the stage dir instead of the user's repo. The per-install command
Expand Down Expand Up @@ -272,7 +368,8 @@ export function materializeSkills(
return {
harness,
installs,
...(installRoot !== undefined ? { sessionInstallRoot: installRoot } : {})
...(installRoot !== undefined ? { sessionInstallRoot: installRoot } : {}),
...(repoRoot !== undefined ? { repoRoot } : {})
};
}

Expand Down Expand Up @@ -349,11 +446,15 @@ export function buildInstallArtifacts(plan: SkillMaterializationPlan): {
// install.installCommand is already self-contained (`sh -c 'cd <root> &&
// …'`) for callers who want to run one at a time, but here we flatten
// to the underlying prpm argv for a cleaner chain.
const providerContext: BuildInstallContext | undefined =
plan.repoRoot !== undefined ? { repoRoot: plan.repoRoot } : undefined;
const perSkill = plan.installs
.map((install) => {
const resolved = resolveSkillSource(install.source);
const provider = providerFor(resolved.kind);
return commandToShellString(provider.buildInstallCommand(resolved, plan.harness));
return commandToShellString(
provider.buildInstallCommand(resolved, plan.harness, providerContext)
);
})
.join(' && ');
const installCommandString = `${scaffold} && cd ${shellEscape(root)} && ${perSkill}`;
Expand Down
Loading
Loading