From 3272387eaad1bde15d56bc76a52f34ea578832d2 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Thu, 12 Feb 2026 19:23:30 -0800 Subject: [PATCH] feat(skills): wire opt-in execution runner selection --- docs/plans/state.json | 13 ++++++- src/cli/skills.test.ts | 84 +++++++++++++++++++++++++++++++++++++++++- src/cli/skills.ts | 68 ++++++++++++++++++++++++++++++---- 3 files changed, 154 insertions(+), 11 deletions(-) diff --git a/docs/plans/state.json b/docs/plans/state.json index f7d4f2f..0d34b2e 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -1460,6 +1460,15 @@ "src/cli/skills.test.ts" ], "test_status": "pnpm typecheck + pnpm test:run src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "installer_cli_runner_opt_in_wiring": { + "status": "completed", + "description": "Added explicit install CLI opt-in wiring for execution (`--execute`) and runner selection (`--runner noop|shell`) with strict confirmation gating, preserving execution-disabled defaults unless explicitly requested", + "files_modified": [ + "src/cli/skills.ts", + "src/cli/skills.test.ts" + ], + "test_status": "pnpm typecheck + pnpm test:run src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" } } } @@ -1488,7 +1497,7 @@ }, "overall_progress": { - "total_test_count": 1545, + "total_test_count": 1548, "all_tests_passing": true, "p0_completion": "3/3 (100%)", "p1_completion": "4/4 (100%)", @@ -1508,7 +1517,7 @@ "gmail_auth_cli": "flynn gmail-auth command implemented with OAuth2 flow, doctor check, config routed to Telegram", "native_audio_support": "completed — smart routing for native audio (Gemini/OpenAI/GitHub) vs Whisper transcription fallback", "remaining_phases_completion": "Phase 1: 3/3 (100%) — context levels, command registry, memory structure. Phase 2: 2/2 (100%) — component registry, confidence routing. Phase 3: 2/2 (100%) — adaptive memory/compaction, truthfulness/autonomy hardening", - "next_up": "Skills infrastructure Phase 3: add explicit CLI opt-in wiring for execution runner selection while preserving execution-disabled defaults and existing confirmation policy gates" + "next_up": "Skills infrastructure Phase 3: extend `skills execute ` to support the same execution opt-in + runner selection path as install while retaining safe defaults" }, "soul_md_and_cron_create": { "date": "2026-02-11", diff --git a/src/cli/skills.test.ts b/src/cli/skills.test.ts index 89ff3ba..7ce0f08 100644 --- a/src/cli/skills.test.ts +++ b/src/cli/skills.test.ts @@ -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 -> ']); + expect(payload.execution.results).toEqual([ + { + installer_type: 'download', + command: 'download https://example.com/install-exec.tgz -> ', + 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')); diff --git a/src/cli/skills.ts b/src/cli/skills.ts index a601a4f..24dcbfc 100644 --- a/src/cli/skills.ts +++ b/src/cli/skills.ts @@ -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 ', 'Installer runner: noop (default) or shell') .option('-c, --config ', '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) {