feat(skills): wire opt-in execution runner selection
This commit is contained in:
+82
-2
@@ -24,6 +24,7 @@ import {
|
||||
runInstallerCommandsWithPolicy,
|
||||
noOpSkillInstallerCommandRunner,
|
||||
createShellSkillInstallerCommandRunner,
|
||||
resolveSkillInstallerCommandRunner,
|
||||
runSkillInstallAction,
|
||||
} from './skills.js';
|
||||
import type { Skill } from '../skills/index.js';
|
||||
@@ -313,7 +314,7 @@ describe('skills CLI helpers', () => {
|
||||
});
|
||||
|
||||
it('builds blocked step envelopes when confirmation is required', () => {
|
||||
const policy = evaluateInstallerExecutionPolicy({ mode: 'install', confirmed: false });
|
||||
const policy = evaluateInstallerExecutionPolicy({ mode: 'install', confirmed: false, executionRequested: true });
|
||||
|
||||
const envelopes = toInstallerExecutionStepEnvelopes(
|
||||
[{ installerType: 'brew', command: 'brew install jq' }],
|
||||
@@ -332,7 +333,7 @@ describe('skills CLI helpers', () => {
|
||||
});
|
||||
|
||||
it('marks install execution policy as confirmation_required when not confirmed', () => {
|
||||
const policy = evaluateInstallerExecutionPolicy({ mode: 'install', confirmed: false });
|
||||
const policy = evaluateInstallerExecutionPolicy({ mode: 'install', confirmed: false, executionRequested: true });
|
||||
|
||||
expect(policy.confirmed).toBe(false);
|
||||
expect(policy.execution_enabled).toBe(false);
|
||||
@@ -347,6 +348,33 @@ describe('skills CLI helpers', () => {
|
||||
expect(policy.reason).toBe('execution_disabled');
|
||||
});
|
||||
|
||||
it('enables install execution only when requested and confirmed', () => {
|
||||
const policy = evaluateInstallerExecutionPolicy({ mode: 'install', confirmed: true, executionRequested: true });
|
||||
|
||||
expect(policy.confirmed).toBe(true);
|
||||
expect(policy.execution_enabled).toBe(true);
|
||||
expect(policy.reason).toBe('execution_enabled');
|
||||
});
|
||||
|
||||
it('resolves installer runner mode and validates invalid values', () => {
|
||||
const noop = resolveSkillInstallerCommandRunner();
|
||||
expect('error' in noop).toBe(false);
|
||||
if (!('error' in noop)) {
|
||||
expect(noop.mode).toBe('noop');
|
||||
expect(noop.runner).toBe(noOpSkillInstallerCommandRunner);
|
||||
}
|
||||
|
||||
const shell = resolveSkillInstallerCommandRunner('shell');
|
||||
expect('error' in shell).toBe(false);
|
||||
if (!('error' in shell)) {
|
||||
expect(shell.mode).toBe('shell');
|
||||
expect(typeof shell.runner.run).toBe('function');
|
||||
}
|
||||
|
||||
const invalid = resolveSkillInstallerCommandRunner('invalid');
|
||||
expect(invalid).toEqual({ error: "Invalid runner 'invalid'. Allowed values: noop, shell." });
|
||||
});
|
||||
|
||||
it('does not invoke command runner when policy disables execution', () => {
|
||||
const runner = {
|
||||
run: vi.fn((_commands: string[]) => [{ command: 'should-not-run', status: 'succeeded' as const }]),
|
||||
@@ -676,6 +704,58 @@ describe('skills CLI helpers', () => {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('runs installer commands when execution is requested and confirmed', () => {
|
||||
const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-'));
|
||||
const sourceDir = join(root, 'source-skill');
|
||||
const managedDir = join(root, 'managed');
|
||||
mkdirSync(sourceDir, { recursive: true });
|
||||
writeFileSync(join(sourceDir, 'SKILL.md'), '# Install Skill\nInstructions');
|
||||
writeFileSync(
|
||||
join(sourceDir, 'manifest.json'),
|
||||
JSON.stringify({
|
||||
name: 'install-skill',
|
||||
description: 'Install',
|
||||
version: '1.0.0',
|
||||
installers: [{ type: 'download', url: 'https://example.com/install-exec.tgz' }],
|
||||
}),
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
const installer = new SkillInstaller(managedDir);
|
||||
const runner = {
|
||||
run: vi.fn((commands: string[]) =>
|
||||
commands.map((command) => ({ command, status: 'succeeded' as const, reason: 'runner_reported_success' })),
|
||||
),
|
||||
};
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined);
|
||||
|
||||
const result = runSkillInstallAction(installer, sourceDir, {
|
||||
mode: 'install',
|
||||
asJson: true,
|
||||
confirmed: true,
|
||||
executionRequested: true,
|
||||
commandRunner: runner,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
expect(runner.run).toHaveBeenCalledTimes(1);
|
||||
const payload = JSON.parse(String(logSpy.mock.calls[logSpy.mock.calls.length - 1]?.[0]));
|
||||
expect(payload.execution.execution_enabled).toBe(true);
|
||||
expect(payload.execution.reason).toBe('execution_enabled');
|
||||
expect(payload.execution.executed).toEqual(['download https://example.com/install-exec.tgz -> <default destination>']);
|
||||
expect(payload.execution.results).toEqual([
|
||||
{
|
||||
installer_type: 'download',
|
||||
command: 'download https://example.com/install-exec.tgz -> <default destination>',
|
||||
status: 'succeeded',
|
||||
reason: 'runner_reported_success',
|
||||
},
|
||||
]);
|
||||
|
||||
logSpy.mockRestore();
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('requires --yes confirmation for uninstall helper', () => {
|
||||
const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-'));
|
||||
const installer = new SkillInstaller(join(root, 'managed'));
|
||||
|
||||
+61
-7
@@ -54,8 +54,9 @@ export interface SkillInstallerExecutionStubView {
|
||||
}
|
||||
|
||||
export type SkillInstallActionMode = 'plan-only' | 'stub' | 'install';
|
||||
export type SkillInstallerExecutionReason = 'execution_disabled' | 'confirmation_required';
|
||||
export type SkillInstallerExecutionReason = 'execution_disabled' | 'confirmation_required' | 'execution_enabled';
|
||||
export type SkillInstallerStepStatus = 'blocked' | 'skipped' | 'succeeded' | 'failed';
|
||||
export type SkillInstallerRunnerMode = 'noop' | 'shell';
|
||||
|
||||
export interface SkillInstallerExecutionPolicy {
|
||||
confirmed: boolean;
|
||||
@@ -130,6 +131,20 @@ export function createShellSkillInstallerCommandRunner(): SkillInstallerCommandR
|
||||
};
|
||||
}
|
||||
|
||||
export function resolveSkillInstallerCommandRunner(
|
||||
mode?: string,
|
||||
): { mode: SkillInstallerRunnerMode; runner: SkillInstallerCommandRunner } | { error: string } {
|
||||
if (!mode || mode === 'noop') {
|
||||
return { mode: 'noop', runner: noOpSkillInstallerCommandRunner };
|
||||
}
|
||||
|
||||
if (mode === 'shell') {
|
||||
return { mode: 'shell', runner: createShellSkillInstallerCommandRunner() };
|
||||
}
|
||||
|
||||
return { error: `Invalid runner '${mode}'. Allowed values: noop, shell.` };
|
||||
}
|
||||
|
||||
export function toInstallerExecutionStepEnvelopes(
|
||||
steps: Array<{ installerType: string; command: string }>,
|
||||
policy: SkillInstallerExecutionPolicy,
|
||||
@@ -371,10 +386,14 @@ export function toSkillInstallerExecutionStubView(skill: Skill): SkillInstallerE
|
||||
|
||||
export function toSkillInstallerExecutionStubFromPreflight(
|
||||
preflight: SkillInstallPreflightView,
|
||||
options?: { mode?: SkillInstallActionMode; confirmed?: boolean },
|
||||
options?: { mode?: SkillInstallActionMode; confirmed?: boolean; executionRequested?: boolean },
|
||||
): SkillInstallerExecutionStubView {
|
||||
const mode = options?.mode ?? 'stub';
|
||||
const policy = evaluateInstallerExecutionPolicy({ mode, confirmed: options?.confirmed ?? false });
|
||||
const policy = evaluateInstallerExecutionPolicy({
|
||||
mode,
|
||||
confirmed: options?.confirmed ?? false,
|
||||
executionRequested: options?.executionRequested ?? false,
|
||||
});
|
||||
const stepEnvelopes = toInstallerExecutionStepEnvelopes(preflight.steps, policy);
|
||||
return {
|
||||
skill: preflight.skill,
|
||||
@@ -394,6 +413,7 @@ export function toSkillInstallerExecutionStubFromPreflight(
|
||||
export function evaluateInstallerExecutionPolicy(opts: {
|
||||
mode: SkillInstallActionMode;
|
||||
confirmed: boolean;
|
||||
executionRequested?: boolean;
|
||||
}): SkillInstallerExecutionPolicy {
|
||||
if (opts.mode === 'install' && !opts.confirmed) {
|
||||
return {
|
||||
@@ -403,6 +423,14 @@ export function evaluateInstallerExecutionPolicy(opts: {
|
||||
};
|
||||
}
|
||||
|
||||
if (opts.mode === 'install' && opts.confirmed && opts.executionRequested) {
|
||||
return {
|
||||
confirmed: true,
|
||||
execution_enabled: true,
|
||||
reason: 'execution_enabled',
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
confirmed: opts.confirmed,
|
||||
execution_enabled: false,
|
||||
@@ -514,6 +542,7 @@ export function runSkillInstallAction(
|
||||
mode: SkillInstallActionMode;
|
||||
asJson: boolean;
|
||||
confirmed: boolean;
|
||||
executionRequested?: boolean;
|
||||
commandRunner?: SkillInstallerCommandRunner;
|
||||
},
|
||||
): { ok: true } | { ok: false; error: string } {
|
||||
@@ -561,12 +590,18 @@ export function runSkillInstallAction(
|
||||
}
|
||||
|
||||
const installPolicy = evaluateInstallerExecutionPolicy({ mode: 'install', confirmed: opts.confirmed });
|
||||
const requestedInstallPolicy = evaluateInstallerExecutionPolicy({
|
||||
mode: 'install',
|
||||
confirmed: opts.confirmed,
|
||||
executionRequested: opts.executionRequested ?? false,
|
||||
});
|
||||
|
||||
const execution =
|
||||
preflight !== null
|
||||
? toSkillInstallerExecutionStubFromPreflight(preflight, {
|
||||
mode: 'install',
|
||||
confirmed: opts.confirmed,
|
||||
executionRequested: opts.executionRequested ?? false,
|
||||
})
|
||||
: {
|
||||
skill: {
|
||||
@@ -588,13 +623,13 @@ export function runSkillInstallAction(
|
||||
|
||||
const commandResults = runInstallerCommandsWithPolicy(
|
||||
execution.wouldRun,
|
||||
installPolicy,
|
||||
requestedInstallPolicy,
|
||||
opts.commandRunner ?? noOpSkillInstallerCommandRunner,
|
||||
);
|
||||
execution.executed = commandResults.map((result) => result.command);
|
||||
execution.results = mergeInstallerExecutionResults(
|
||||
execution.attempted,
|
||||
installPolicy,
|
||||
requestedInstallPolicy,
|
||||
commandResults,
|
||||
);
|
||||
|
||||
@@ -708,10 +743,20 @@ export function registerSkillsCommand(program: Command): void {
|
||||
.option('--json', 'Output preflight and install result as JSON')
|
||||
.option('--preflight-only', 'Show installer preflight without performing install')
|
||||
.option('--stub', 'Show installer execution stub without performing install')
|
||||
.option('--confirm', 'Mark installer execution intent as confirmed (execution remains disabled)')
|
||||
.option('--confirm', 'Mark installer execution intent as confirmed (required with --execute)')
|
||||
.option('--execute', 'Enable installer command execution (requires --confirm)')
|
||||
.option('--runner <mode>', 'Installer runner: noop (default) or shell')
|
||||
.option('-c, --config <path>', 'Config file path')
|
||||
.action(
|
||||
(pathArg: string, opts: { json?: boolean; preflightOnly?: boolean; stub?: boolean; confirm?: boolean; config?: string }) => {
|
||||
(pathArg: string, opts: {
|
||||
json?: boolean;
|
||||
preflightOnly?: boolean;
|
||||
stub?: boolean;
|
||||
confirm?: boolean;
|
||||
execute?: boolean;
|
||||
runner?: string;
|
||||
config?: string;
|
||||
}) => {
|
||||
const loaded = loadConfigSafe(opts.config);
|
||||
if (loaded.error || !loaded.config) {
|
||||
console.error(loaded.error ?? 'Failed to load config');
|
||||
@@ -719,6 +764,13 @@ export function registerSkillsCommand(program: Command): void {
|
||||
return;
|
||||
}
|
||||
|
||||
const runnerResolution = resolveSkillInstallerCommandRunner(opts.runner);
|
||||
if ('error' in runnerResolution) {
|
||||
console.error(runnerResolution.error);
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
|
||||
const defaultManagedDir = resolve(homedir(), '.flynn/workspace/skills');
|
||||
const installer = new SkillInstaller(loaded.config.skills.managed_dir ?? defaultManagedDir);
|
||||
|
||||
@@ -727,6 +779,8 @@ export function registerSkillsCommand(program: Command): void {
|
||||
mode,
|
||||
asJson: opts.json ?? false,
|
||||
confirmed: opts.confirm ?? false,
|
||||
executionRequested: opts.execute ?? false,
|
||||
commandRunner: runnerResolution.runner,
|
||||
});
|
||||
|
||||
if (!result.ok) {
|
||||
|
||||
Reference in New Issue
Block a user