From 8affe8bea982bd1d997d9fc36e617b7451bd62e5 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Thu, 12 Feb 2026 19:28:44 -0800 Subject: [PATCH] feat(skills): add execute command opt-in runner flow --- docs/plans/state.json | 13 ++++++-- src/cli/skills.test.ts | 57 +++++++++++++++++++++++++++++++++ src/cli/skills.ts | 72 +++++++++++++++++++++++++++++++++++------- 3 files changed, 128 insertions(+), 14 deletions(-) diff --git a/docs/plans/state.json b/docs/plans/state.json index 0d34b2e..ba17e60 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -1469,6 +1469,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" + }, + "skills_execute_opt_in_runner_wiring": { + "status": "completed", + "description": "Extended `skills execute ` with explicit execution opt-in and runner selection (`--execute`, `--runner noop|shell`), wired through shared policy/result envelope logic with safe defaults and confirmation gating", + "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" } } } @@ -1497,7 +1506,7 @@ }, "overall_progress": { - "total_test_count": 1548, + "total_test_count": 1550, "all_tests_passing": true, "p0_completion": "3/3 (100%)", "p1_completion": "4/4 (100%)", @@ -1517,7 +1526,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: extend `skills execute ` to support the same execution opt-in + runner selection path as install while retaining safe defaults" + "next_up": "Skills infrastructure Phase 3: add CLI-level integration tests for `skills install/execute` option parsing (`--execute`, `--runner`, `--confirm`) to lock behavior and failure modes" }, "soul_md_and_cron_create": { "date": "2026-02-11", diff --git a/src/cli/skills.test.ts b/src/cli/skills.test.ts index 7ce0f08..19472a7 100644 --- a/src/cli/skills.test.ts +++ b/src/cli/skills.test.ts @@ -25,6 +25,7 @@ import { noOpSkillInstallerCommandRunner, createShellSkillInstallerCommandRunner, resolveSkillInstallerCommandRunner, + runSkillExecuteAction, runSkillInstallAction, } from './skills.js'; import type { Skill } from '../skills/index.js'; @@ -277,6 +278,26 @@ describe('skills CLI helpers', () => { expect(output).toContain('Would run:'); expect(output).toContain('- brew install jq'); expect(output).toContain('Skipped:'); + expect(output).toContain('Results:'); + }); + + it('renders execution report text when commands are executed', () => { + const output = renderSkillInstallerExecutionStub({ + skill: { name: 'exec-stub', tier: 'bundled', version: '1.0.0' }, + execution: 'stub', + mode: 'stub', + confirmed: true, + execution_enabled: true, + executed: ['brew install jq'], + reason: 'execution_enabled', + attempted: [{ installer_type: 'brew', command: 'brew install jq' }], + results: [{ installer_type: 'brew', command: 'brew install jq', status: 'succeeded', reason: 'runner_reported_success' }], + wouldRun: ['brew install jq'], + skipped: [], + }); + + expect(output).toContain('Installer commands were executed.'); + expect(output).toContain('[brew] succeeded brew install jq (runner_reported_success)'); }); it('derives execution stub view from preflight data', () => { @@ -756,6 +777,42 @@ describe('skills CLI helpers', () => { rmSync(root, { recursive: true, force: true }); }); + it('execute action honors opt-in execution and runner selection', () => { + const skill = buildSkill({ + manifest: { + name: 'execute-skill', + description: 'Execute me', + version: '1.0.0', + tier: 'managed', + installers: [{ type: 'download', url: 'https://example.com/execute.tgz' }], + }, + }); + + 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 = runSkillExecuteAction(skill, { + asJson: true, + confirmed: true, + executionRequested: true, + commandRunner: runner, + }); + + expect(result.ok).toBe(true); + expect(result.execution.execution_enabled).toBe(true); + expect(result.execution.reason).toBe('execution_enabled'); + expect(runner.run).toHaveBeenCalledTimes(1); + const payload = JSON.parse(String(logSpy.mock.calls[0]?.[0])); + expect(payload.execution_enabled).toBe(true); + expect(payload.executed).toEqual(['download https://example.com/execute.tgz -> ']); + + logSpy.mockRestore(); + }); + 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 24dcbfc..06ab014 100644 --- a/src/cli/skills.ts +++ b/src/cli/skills.ts @@ -453,7 +453,7 @@ export function runInstallerCommandsWithPolicy( export function renderSkillInstallerExecutionStub(view: SkillInstallerExecutionStubView): string { const lines: string[] = [ `Installer execution stub for '${view.skill.name}' (${view.skill.tier}, v${view.skill.version})`, - 'No installer commands were executed.', + view.executed.length > 0 ? 'Installer commands were executed.' : 'No installer commands were executed.', ]; if (view.wouldRun.length === 0) { @@ -472,9 +472,53 @@ export function renderSkillInstallerExecutionStub(view: SkillInstallerExecutionS } } + if (view.results.length > 0) { + lines.push('Results:'); + for (const result of view.results) { + lines.push(`- [${result.installer_type}] ${result.status} ${result.command} (${result.reason})`); + } + } + return lines.join('\n'); } +export function runSkillExecuteAction( + skill: Skill, + opts: { + asJson: boolean; + confirmed: boolean; + executionRequested?: boolean; + commandRunner?: SkillInstallerCommandRunner; + }, +): { ok: true; execution: SkillInstallerExecutionStubView } { + const execution = toSkillInstallerExecutionStubView(skill); + const policy = evaluateInstallerExecutionPolicy({ + mode: 'install', + confirmed: opts.confirmed, + executionRequested: opts.executionRequested ?? false, + }); + + execution.confirmed = policy.confirmed; + execution.execution_enabled = policy.execution_enabled; + execution.reason = policy.reason; + + const commandResults = runInstallerCommandsWithPolicy( + execution.wouldRun, + policy, + opts.commandRunner ?? noOpSkillInstallerCommandRunner, + ); + execution.executed = commandResults.map((result) => result.command); + execution.results = mergeInstallerExecutionResults(execution.attempted, policy, commandResults); + + if (opts.asJson) { + console.log(JSON.stringify(execution, null, 2)); + } else { + console.log(renderSkillInstallerExecutionStub(execution)); + } + + return { ok: true, execution }; +} + export function summarizeSkillsRefresh(skills: Skill[]): SkillRefreshSummary { const summary: SkillRefreshSummary = { total: skills.length, @@ -880,11 +924,13 @@ export function registerSkillsCommand(program: Command): void { skills .command('execute ') - .description('Preview installer execution steps (stub only; no commands run)') + .description('Preview or execute installer steps for an installed skill') .option('--json', 'Output as JSON') - .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((name: string, opts: { json?: boolean; confirm?: boolean; config?: string }) => { + .action((name: string, opts: { json?: boolean; confirm?: boolean; execute?: boolean; runner?: string; config?: string }) => { const loaded = loadSkillsFromConfig(opts.config); if (loaded.error || !loaded.skills) { console.error(loaded.error ?? 'Failed to load skills'); @@ -899,16 +945,18 @@ export function registerSkillsCommand(program: Command): void { return; } - const view = toSkillInstallerExecutionStubView(skill); - const policy = evaluateInstallerExecutionPolicy({ mode: 'stub', confirmed: opts.confirm ?? false }); - view.confirmed = policy.confirmed; - view.execution_enabled = policy.execution_enabled; - view.reason = policy.reason; - if (opts.json) { - console.log(JSON.stringify(view, null, 2)); + const runnerResolution = resolveSkillInstallerCommandRunner(opts.runner); + if ('error' in runnerResolution) { + console.error(runnerResolution.error); + process.exitCode = 1; return; } - console.log(renderSkillInstallerExecutionStub(view)); + runSkillExecuteAction(skill, { + asJson: opts.json ?? false, + confirmed: opts.confirm ?? false, + executionRequested: opts.execute ?? false, + commandRunner: runnerResolution.runner, + }); }); }