diff --git a/src/discovery.ts b/src/discovery.ts index 7156ea6a..62dd63bb 100644 --- a/src/discovery.ts +++ b/src/discovery.ts @@ -20,7 +20,8 @@ import type { ManifestEntry } from './build-manifest.js'; /** Plugins directory: ~/.opencli/plugins/ */ export const PLUGINS_DIR = path.join(os.homedir(), '.opencli', 'plugins'); -const CLI_MODULE_PATTERN = /\bcli\s*\(/; +/** Matches files that register commands via cli() or lifecycle hooks */ +const PLUGIN_MODULE_PATTERN = /\b(?:cli|onStartup|onBeforeExecute|onAfterExecute)\s*\(/; import type { YamlCliDefinition } from './yaml-schema.js'; @@ -246,7 +247,7 @@ async function discoverPluginDir(dir: string, site: string): Promise { async function isCliModule(filePath: string): Promise { try { const source = await fs.promises.readFile(filePath, 'utf-8'); - return CLI_MODULE_PATTERN.test(source); + return PLUGIN_MODULE_PATTERN.test(source); } catch (err) { log.warn(`Failed to inspect module ${filePath}: ${getErrorMessage(err)}`); return false; diff --git a/src/engine.test.ts b/src/engine.test.ts index ff8066c5..30e5da3f 100644 --- a/src/engine.test.ts +++ b/src/engine.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { discoverClis, discoverPlugins, PLUGINS_DIR } from './discovery.js'; import { executeCommand } from './execution.js'; import { getRegistry, cli, Strategy } from './registry.js'; +import { clearAllHooks, onAfterExecute } from './hooks.js'; import * as fs from 'node:fs'; import * as path from 'node:path'; @@ -118,6 +119,10 @@ columns: [message] }); describe('executeCommand', () => { + beforeEach(() => { + clearAllHooks(); + }); + it('accepts kebab-case option names after Commander camelCases them', async () => { const cmd = cli({ site: 'test-engine', @@ -195,4 +200,28 @@ describe('executeCommand', () => { await executeCommand(cmd, {}, true); expect(receivedDebug).toBe(true); }); + + it('fires onAfterExecute even when command execution throws', async () => { + const seen: Array<{ error?: unknown; finishedAt?: number }> = []; + onAfterExecute((ctx) => { + seen.push({ error: ctx.error, finishedAt: ctx.finishedAt }); + }); + + const cmd = cli({ + site: 'test-engine', + name: 'failing-test', + description: 'failing command', + browser: false, + strategy: Strategy.PUBLIC, + func: async () => { + throw new Error('boom'); + }, + }); + + await expect(executeCommand(cmd, {})).rejects.toThrow('boom'); + expect(seen).toHaveLength(1); + expect(seen[0].error).toBeInstanceOf(Error); + expect((seen[0].error as Error).message).toBe('boom'); + expect(typeof seen[0].finishedAt).toBe('number'); + }); }); diff --git a/src/execution.ts b/src/execution.ts index 36d68858..2d9109ce 100644 --- a/src/execution.ts +++ b/src/execution.ts @@ -7,6 +7,7 @@ * 3. Domain pre-navigation for cookie/header strategies * 4. Timeout enforcement * 5. Lazy-loading of TS modules from manifest + * 6. Lifecycle hooks (onBeforeExecute / onAfterExecute) */ import { type CliCommand, type InternalCliCommand, type Arg, Strategy, getRegistry, fullName } from './registry.js'; @@ -16,6 +17,7 @@ import { executePipeline } from './pipeline/index.js'; import { AdapterLoadError, ArgumentError, CommandExecutionError, getErrorMessage } from './errors.js'; import { shouldUseBrowserSession } from './capabilityRouting.js'; import { getBrowserFactory, browserSession, runWithTimeout, DEFAULT_BROWSER_COMMAND_TIMEOUT } from './runtime.js'; +import { emitHook, type HookContext } from './hooks.js'; /** Set of TS module paths that have been loaded */ const _loadedModules = new Set(); @@ -138,6 +140,9 @@ function resolvePreNav(cmd: CliCommand): string | null { * * This is the unified entry point — callers don't need to care about * whether the command requires a browser or not. + * + * Lifecycle hooks are emitted around execution: + * onBeforeExecute → command runs → onAfterExecute(result) */ export async function executeCommand( cmd: CliCommand, @@ -152,24 +157,44 @@ export async function executeCommand( throw new ArgumentError(getErrorMessage(err)); } - if (shouldUseBrowserSession(cmd)) { - const BrowserFactory = getBrowserFactory(); - return browserSession(BrowserFactory, async (page) => { - // Pre-navigate to target domain for cookie/header context if needed. - // Each adapter controls this via `navigateBefore` (see CliCommand docs). - const preNavUrl = resolvePreNav(cmd); - if (preNavUrl) { - try { await page.goto(preNavUrl); await page.wait(2); } catch (err) { - if (debug) console.error(`[pre-nav] Failed to navigate to ${preNavUrl}: ${err instanceof Error ? err.message : err}`); + // Build hook context shared across onBeforeExecute and onAfterExecute + const hookCtx: HookContext = { + command: fullName(cmd), + args: kwargs, + startedAt: Date.now(), + }; + await emitHook('onBeforeExecute', hookCtx); + + let result: unknown; + try { + if (shouldUseBrowserSession(cmd)) { + const BrowserFactory = getBrowserFactory(); + result = await browserSession(BrowserFactory, async (page) => { + // Pre-navigate to target domain for cookie/header context if needed. + // Each adapter controls this via `navigateBefore` (see CliCommand docs). + const preNavUrl = resolvePreNav(cmd); + if (preNavUrl) { + try { await page.goto(preNavUrl); await page.wait(2); } catch (err) { + if (debug) console.error(`[pre-nav] Failed to navigate to ${preNavUrl}: ${err instanceof Error ? err.message : err}`); + } } - } - return runWithTimeout(runCommand(cmd, page, kwargs, debug), { - timeout: cmd.timeoutSeconds ?? DEFAULT_BROWSER_COMMAND_TIMEOUT, - label: fullName(cmd), - }); - }, { workspace: `site:${cmd.site}` }); + return runWithTimeout(runCommand(cmd, page, kwargs, debug), { + timeout: cmd.timeoutSeconds ?? DEFAULT_BROWSER_COMMAND_TIMEOUT, + label: fullName(cmd), + }); + }, { workspace: `site:${cmd.site}` }); + } else { + // Non-browser commands run directly + result = await runCommand(cmd, null, kwargs, debug); + } + } catch (err) { + hookCtx.error = err; + hookCtx.finishedAt = Date.now(); + await emitHook('onAfterExecute', hookCtx); + throw err; } - // Non-browser commands run directly - return runCommand(cmd, null, kwargs, debug); + hookCtx.finishedAt = Date.now(); + await emitHook('onAfterExecute', hookCtx, result); + return result; } diff --git a/src/hooks.test.ts b/src/hooks.test.ts new file mode 100644 index 00000000..4ab4afd3 --- /dev/null +++ b/src/hooks.test.ts @@ -0,0 +1,126 @@ +/** + * Tests for the plugin lifecycle hooks system. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { + onStartup, + onBeforeExecute, + onAfterExecute, + emitHook, + clearAllHooks, + type HookContext, +} from './hooks.js'; + +beforeEach(() => { + clearAllHooks(); +}); + +describe('hook registration and emission', () => { + it('onBeforeExecute hook is called with context', async () => { + const calls: HookContext[] = []; + onBeforeExecute((ctx) => { calls.push({ ...ctx }); }); + + await emitHook('onBeforeExecute', { command: 'test/cmd', args: { limit: 5 }, startedAt: 100 }); + + expect(calls).toHaveLength(1); + expect(calls[0].command).toBe('test/cmd'); + expect(calls[0].args).toEqual({ limit: 5 }); + expect(calls[0].startedAt).toBe(100); + }); + + it('onAfterExecute hook receives result', async () => { + const results: unknown[] = []; + onAfterExecute((_ctx, result) => { results.push(result); }); + + const mockResult = [{ title: 'item1' }, { title: 'item2' }]; + await emitHook('onAfterExecute', { command: 'test/cmd', args: {} }, mockResult); + + expect(results).toHaveLength(1); + expect(results[0]).toEqual(mockResult); + }); + + it('onStartup hook fires', async () => { + let fired = false; + onStartup(() => { fired = true; }); + + await emitHook('onStartup', { command: '__startup__', args: {} }); + expect(fired).toBe(true); + }); + + it('multiple hooks on the same event fire in order', async () => { + const order: number[] = []; + onBeforeExecute(() => { order.push(1); }); + onBeforeExecute(() => { order.push(2); }); + onBeforeExecute(() => { order.push(3); }); + + await emitHook('onBeforeExecute', { command: 'test/cmd', args: {} }); + expect(order).toEqual([1, 2, 3]); + }); + + it('async hooks are awaited', async () => { + const order: string[] = []; + onBeforeExecute(async () => { + await new Promise((r) => setTimeout(r, 10)); + order.push('async-done'); + }); + onBeforeExecute(() => { order.push('sync'); }); + + await emitHook('onBeforeExecute', { command: 'test/cmd', args: {} }); + expect(order).toEqual(['async-done', 'sync']); + }); +}); + +describe('hook error isolation', () => { + it('failing hook does not prevent other hooks from running', async () => { + const calls: string[] = []; + + onBeforeExecute(() => { calls.push('first'); }); + onBeforeExecute(() => { throw new Error('boom'); }); + onBeforeExecute(() => { calls.push('third'); }); + + await emitHook('onBeforeExecute', { command: 'test/cmd', args: {} }); + + // First and third should still run despite the second throwing + expect(calls).toEqual(['first', 'third']); + }); + + it('async hook rejection does not prevent other hooks', async () => { + const calls: string[] = []; + + onAfterExecute(() => { calls.push('before-reject'); }); + onAfterExecute(async () => { throw new Error('async boom'); }); + onAfterExecute(() => { calls.push('after-reject'); }); + + await emitHook('onAfterExecute', { command: 'test/cmd', args: {} }, null); + + expect(calls).toEqual(['before-reject', 'after-reject']); + }); +}); + +describe('no-op when no hooks registered', () => { + it('emitHook with no registered hooks does nothing', async () => { + // Should not throw + await emitHook('onBeforeExecute', { command: 'test/cmd', args: {} }); + await emitHook('onAfterExecute', { command: 'test/cmd', args: {} }, []); + await emitHook('onStartup', { command: '__startup__', args: {} }); + }); +}); + +describe('clearAllHooks', () => { + it('removes all hooks', async () => { + let called = false; + onStartup(() => { called = true; }); + + clearAllHooks(); + await emitHook('onStartup', { command: '__startup__', args: {} }); + + expect(called).toBe(false); + }); +}); + +describe('globalThis singleton', () => { + it('uses globalThis.__opencli_hooks__ for shared state', () => { + expect(globalThis.__opencli_hooks__).toBeInstanceOf(Map); + }); +}); diff --git a/src/hooks.ts b/src/hooks.ts new file mode 100644 index 00000000..b0123ae4 --- /dev/null +++ b/src/hooks.ts @@ -0,0 +1,90 @@ +/** + * Plugin lifecycle hooks: allows plugins to tap into opencli's execution lifecycle. + * + * Hooks use globalThis (like the command registry) to guarantee a single shared + * instance across all module copies — critical when TS plugins are loaded via + * npm link / peerDependency symlinks. + * + * Available hooks: + * onStartup — fired once after all commands & plugins are discovered + * onBeforeExecute — fired before every command execution + * onAfterExecute — fired after every command execution (receives result) + */ + +import { log } from './logger.js'; + +export type HookName = 'onStartup' | 'onBeforeExecute' | 'onAfterExecute'; + +export interface HookContext { + /** Command full name in "site/name" format, or "__startup__" for onStartup */ + command: string; + /** Coerced and validated arguments */ + args: Record; + /** Epoch ms when execution started (set by executeCommand) */ + startedAt?: number; + /** Epoch ms when execution finished (set by executeCommand) */ + finishedAt?: number; + /** Error thrown by the command, if execution failed */ + error?: unknown; + /** Plugins can attach arbitrary data here for cross-hook communication */ + [key: string]: unknown; +} + +export type HookFn = (ctx: HookContext, result?: unknown) => void | Promise; + +// ── Singleton hook store (shared across module instances via globalThis) ── +declare global { + // eslint-disable-next-line no-var + var __opencli_hooks__: Map | undefined; +} +const _hooks: Map = + globalThis.__opencli_hooks__ ??= new Map(); + +// ── Registration API (used by plugins) ───────────────────────────────────── + +function addHook(name: HookName, fn: HookFn): void { + const list = _hooks.get(name) ?? []; + list.push(fn); + _hooks.set(name, list); +} + +/** Register a hook that fires once after all plugins are discovered. */ +export function onStartup(fn: HookFn): void { + addHook('onStartup', fn); +} + +/** Register a hook that fires before every command execution. */ +export function onBeforeExecute(fn: HookFn): void { + addHook('onBeforeExecute', fn); +} + +/** Register a hook that fires after every command execution with the result. */ +export function onAfterExecute(fn: HookFn): void { + addHook('onAfterExecute', fn); +} + +// ── Emit API (used internally by opencli core) ───────────────────────────── + +/** + * Trigger all registered handlers for a hook. + * Each handler is wrapped in try/catch — a failing hook never blocks command execution. + */ +export async function emitHook(name: HookName, ctx: HookContext, result?: unknown): Promise { + const handlers = _hooks.get(name); + if (!handlers || handlers.length === 0) return; + + for (const fn of handlers) { + try { + await fn(ctx, result); + } catch (err) { + log.warn(`Hook ${name} handler failed: ${err instanceof Error ? err.message : String(err)}`); + } + } +} + +/** + * Remove all registered hooks. Intended for testing only. + */ +export function clearAllHooks(): void { + _hooks.clear(); +} diff --git a/src/main.ts b/src/main.ts index 59ee5896..3ac704e5 100644 --- a/src/main.ts +++ b/src/main.ts @@ -19,6 +19,7 @@ import { fileURLToPath } from 'node:url'; import { discoverClis, discoverPlugins } from './discovery.js'; import { getCompletions } from './completion.js'; import { runCli } from './cli.js'; +import { emitHook } from './hooks.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -49,4 +50,5 @@ if (getCompIdx !== -1) { process.exit(0); } +await emitHook('onStartup', { command: '__startup__', args: {} }); runCli(BUILTIN_CLIS, USER_CLIS); diff --git a/src/registry-api.ts b/src/registry-api.ts index 64da785d..c5791f65 100644 --- a/src/registry-api.ts +++ b/src/registry-api.ts @@ -10,3 +10,5 @@ export { cli, Strategy, getRegistry, fullName, registerCommand } from './registry.js'; export type { CliCommand, Arg, CliOptions } from './registry.js'; export type { IPage } from './types.js'; +export { onStartup, onBeforeExecute, onAfterExecute } from './hooks.js'; +export type { HookFn, HookContext, HookName } from './hooks.js';