From 48608863ea5c96ae65cc52f24e3bb001b122d039 Mon Sep 17 00:00:00 2001 From: Naomi <119312416+Naominour@users.noreply.github.com> Date: Mon, 15 Jun 2026 19:01:21 +0100 Subject: [PATCH] feat: add dry-run mode for hook execution (#2116) (#2188) - Global --dry-run flag and ECC_DRY_RUN=1 env var - Enriched preview: shows target file path, tool name, and command - --dry-run stripped from argv so command routing works correctly - Handles non-JSON and empty stdin gracefully (session/stop hooks) - 10 tests covering isDryRun(), hook gating, enriched output, CLI routing --- scripts/ecc.js | 20 ++- scripts/hooks/run-with-flags.js | 53 ++++++- scripts/lib/hook-flags.js | 5 + tests/lib/dry-run.test.js | 250 ++++++++++++++++++++++++++++++++ 4 files changed, 326 insertions(+), 2 deletions(-) create mode 100644 tests/lib/dry-run.test.js diff --git a/scripts/ecc.js b/scripts/ecc.js index 946a1fc7..7e38b3d3 100755 --- a/scripts/ecc.js +++ b/scripts/ecc.js @@ -106,6 +106,7 @@ ECC selective-install CLI Usage: ecc [args...] ecc [install args...] + ecc --dry-run [args...] Commands: ${PRIMARY_COMMANDS.map(command => ` ${command.padEnd(15)} ${COMMANDS[command].description}`).join('\n')} @@ -115,6 +116,9 @@ Compatibility: ecc [args...] Without a command, args are routed to "install" ecc help Show help for a specific command +Global Flags: + --dry-run Preview actions without executing (sets ECC_DRY_RUN=1) + Examples: ecc typescript ecc install --profile developer --target claude @@ -152,7 +156,21 @@ function resolveCommand(argv) { return { mode: 'help' }; } - const [firstArg, ...restArgs] = args; + if (args.includes('--dry-run')) { + process.env.ECC_DRY_RUN = '1'; + } + + let cmdStart = 0; + while (cmdStart < args.length && args[cmdStart] === '--dry-run') { + cmdStart++; + } + + if (cmdStart >= args.length) { + return { mode: 'help' }; + } + + const firstArg = args[cmdStart]; + const restArgs = args.slice(cmdStart + 1); if (firstArg === '--help' || firstArg === '-h') { return { mode: 'help' }; diff --git a/scripts/hooks/run-with-flags.js b/scripts/hooks/run-with-flags.js index c1c3b904..41ff1462 100755 --- a/scripts/hooks/run-with-flags.js +++ b/scripts/hooks/run-with-flags.js @@ -11,7 +11,7 @@ const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); -const { isHookEnabled } = require('../lib/hook-flags'); +const { isHookEnabled, isDryRun } = require('../lib/hook-flags'); const { buildPreToolUseAdditionalContext } = require('./pretooluse-visible-output'); const MAX_STDIN = 1024 * 1024; @@ -100,6 +100,50 @@ function getPluginRoot() { return path.resolve(__dirname, '..', '..'); } + +//Safely extract target context from hook stdin JSON for dry-run preview. + +function extractTargetContext(raw) { + const result = { tool: '', filePath: '', command: '' }; + if (!raw || typeof raw !== 'string') return result; + + try { + const payload = JSON.parse(raw); + if (payload && typeof payload === 'object') { + result.tool = String(payload.tool || ''); + const input = payload.tool_input; + if (input && typeof input === 'object') { + result.filePath = String(input.file_path || input.path || ''); + result.command = String(input.command || ''); + } + } + } catch { + } + return result; +} + +// Build the [DryRun] preview line for stderr. + +function buildDryRunPreview(hookId, relScriptPath, profilesCsv, raw) { + const ctx = extractTargetContext(raw); + const parts = [ + `[DryRun] Hook "${hookId}" would execute: ${relScriptPath}`, + `(enabled=true, profiles=${profilesCsv || 'default'})`, + ]; + + if (ctx.tool) { + parts.push(`tool=${ctx.tool}`); + } + if (ctx.filePath) { + parts.push(`target=${ctx.filePath}`); + } + if (ctx.command) { + parts.push(`command=${ctx.command}`); + } + + return parts.join(' ') + '\n'; +} + async function main() { const [, , hookId, relScriptPath, profilesCsv] = process.argv; const { raw, truncated } = await readStdinRaw(); @@ -125,6 +169,13 @@ async function main() { return; } + if (isDryRun()) { + const preview = buildDryRunPreview(hookId, relScriptPath, profilesCsv, raw); + process.stderr.write(preview); + process.stdout.write(raw); + process.exit(0); + } + const pluginRoot = getPluginRoot(); const resolvedRoot = path.resolve(pluginRoot); const scriptPath = path.resolve(pluginRoot, relScriptPath); diff --git a/scripts/lib/hook-flags.js b/scripts/lib/hook-flags.js index 4f56660d..70106bc1 100644 --- a/scripts/lib/hook-flags.js +++ b/scripts/lib/hook-flags.js @@ -50,6 +50,10 @@ function parseProfiles(rawProfiles, fallback = ['standard', 'strict']) { return parsed.length > 0 ? parsed : [...fallback]; } +function isDryRun() { + return process.env.ECC_DRY_RUN === '1'; +} + function isHookEnabled(hookId, options = {}) { const id = normalizeId(hookId); if (!id) return true; @@ -71,4 +75,5 @@ module.exports = { getDisabledHookIds, parseProfiles, isHookEnabled, + isDryRun, }; diff --git a/tests/lib/dry-run.test.js b/tests/lib/dry-run.test.js new file mode 100644 index 00000000..18190ea9 --- /dev/null +++ b/tests/lib/dry-run.test.js @@ -0,0 +1,250 @@ +/** + * Tests for dry-run mode + * + * Run with: node tests/lib/dry-run.test.js + */ + +const assert = require('assert'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + return true; + } catch (err) { + console.log(` ✗ ${name}`); + console.log(` Error: ${err.message}`); + return false; + } +} + +function runTests() { + console.log('\n=== Testing dry-run mode ===\n'); + + let passed = 0; + let failed = 0; + + console.log('isDryRun():'); + + if (test('returns false when ECC_DRY_RUN is unset', () => { + const env = { ...process.env }; + delete env.ECC_DRY_RUN; + const result = spawnSync(process.execPath, [ + '-e', + 'const { isDryRun } = require("./scripts/lib/hook-flags"); process.exit(isDryRun() ? 1 : 0)', + ], { cwd: path.resolve(__dirname, '..', '..'), env }); + assert.strictEqual(result.status, 0); + })) passed++; else failed++; + + if (test('returns true when ECC_DRY_RUN=1', () => { + const env = { ...process.env, ECC_DRY_RUN: '1' }; + const result = spawnSync(process.execPath, [ + '-e', + 'const { isDryRun } = require("./scripts/lib/hook-flags"); process.exit(isDryRun() ? 1 : 0)', + ], { cwd: path.resolve(__dirname, '..', '..'), env }); + assert.strictEqual(result.status, 1); + })) passed++; else failed++; + + if (test('returns false when ECC_DRY_RUN=0', () => { + const env = { ...process.env, ECC_DRY_RUN: '0' }; + const result = spawnSync(process.execPath, [ + '-e', + 'const { isDryRun } = require("./scripts/lib/hook-flags"); process.exit(isDryRun() ? 1 : 0)', + ], { cwd: path.resolve(__dirname, '..', '..'), env }); + assert.strictEqual(result.status, 0); + })) passed++; else failed++; + + console.log('\nrun-with-flags.js dry-run gating:'); + + if (test('skips hook execution and logs preview when ECC_DRY_RUN=1', () => { + const runWithFlags = path.resolve(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js'); + const hookScript = 'scripts/hooks/doc-file-warning.js'; + const input = JSON.stringify({ tool: 'Write', tool_input: { file_path: '/tmp/test.md' } }); + + const result = spawnSync(process.execPath, [ + runWithFlags, + 'pre:write:doc-file-warning', + hookScript, + 'standard,strict', + ], { + input, + encoding: 'utf8', + env: { ...process.env, ECC_DRY_RUN: '1' }, + cwd: path.resolve(__dirname, '..', '..'), + }); + + assert.strictEqual(result.status, 0, `Expected exit 0, got ${result.status}`); + assert.ok( + result.stderr.includes('[DryRun]'), + `Expected stderr to contain [DryRun] tag, got: ${result.stderr}` + ); + assert.ok( + result.stderr.includes('pre:write:doc-file-warning'), + `Expected stderr to contain hook ID, got: ${result.stderr}` + ); + assert.ok( + result.stderr.includes('tool=Write'), + `Expected stderr to contain tool name, got: ${result.stderr}` + ); + assert.ok( + result.stderr.includes('target=/tmp/test.md'), + `Expected stderr to contain target file path, got: ${result.stderr}` + ); + assert.strictEqual(result.stdout, input, 'Expected stdin to be passed through unchanged'); + })) passed++; else failed++; + + if (test('dry-run preview includes command for bash hooks', () => { + const runWithFlags = path.resolve(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js'); + const hookScript = 'scripts/hooks/block-no-verify.js'; + const input = JSON.stringify({ tool: 'Bash', tool_input: { command: 'git commit --no-verify -m "test"' } }); + + const result = spawnSync(process.execPath, [ + runWithFlags, + 'pre:bash:block-no-verify', + hookScript, + 'standard,strict', + ], { + input, + encoding: 'utf8', + env: { ...process.env, ECC_DRY_RUN: '1' }, + cwd: path.resolve(__dirname, '..', '..'), + }); + + assert.strictEqual(result.status, 0, `Expected exit 0, got ${result.status}`); + assert.ok( + result.stderr.includes('tool=Bash'), + `Expected stderr to contain tool=Bash, got: ${result.stderr}` + ); + assert.ok( + result.stderr.includes('command=git commit --no-verify'), + `Expected stderr to contain command, got: ${result.stderr}` + ); + assert.strictEqual(result.stdout, input, 'Expected stdin to be passed through unchanged'); + })) passed++; else failed++; + + if (test('dry-run preview handles non-JSON stdin gracefully', () => { + const runWithFlags = path.resolve(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js'); + const hookScript = 'scripts/hooks/session-start.js'; + const input = 'not valid json at all'; + + const result = spawnSync(process.execPath, [ + runWithFlags, + 'pre:session:start', + hookScript, + 'standard', + ], { + input, + encoding: 'utf8', + env: { ...process.env, ECC_DRY_RUN: '1' }, + cwd: path.resolve(__dirname, '..', '..'), + }); + + assert.strictEqual(result.status, 0, `Expected exit 0, got ${result.status}`); + assert.ok( + result.stderr.includes('[DryRun]'), + `Expected stderr to contain [DryRun], got: ${result.stderr}` + ); + assert.ok( + !result.stderr.includes('tool='), + 'Expected no tool= when stdin is not JSON' + ); + assert.strictEqual(result.stdout, input, 'Expected stdin to be passed through unchanged'); + })) passed++; else failed++; + + if (test('dry-run preview handles empty stdin gracefully', () => { + const runWithFlags = path.resolve(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js'); + const hookScript = 'scripts/hooks/session-start.js'; + + const result = spawnSync(process.execPath, [ + runWithFlags, + 'post:stop:session-end', + hookScript, + 'standard', + ], { + input: '', + encoding: 'utf8', + env: { ...process.env, ECC_DRY_RUN: '1' }, + cwd: path.resolve(__dirname, '..', '..'), + }); + + assert.strictEqual(result.status, 0, `Expected exit 0, got ${result.status}`); + assert.ok( + result.stderr.includes('[DryRun]'), + `Expected stderr to contain [DryRun], got: ${result.stderr}` + ); + assert.ok( + !result.stderr.includes('target='), + 'Expected no target= when stdin is empty' + ); + })) passed++; else failed++; + + if (test('executes hook normally when ECC_DRY_RUN is not set', () => { + const runWithFlags = path.resolve(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js'); + const hookScript = 'scripts/hooks/doc-file-warning.js'; + const input = JSON.stringify({ tool: 'Write', tool_input: { file_path: '/tmp/test.txt' } }); + + const env = { ...process.env }; + delete env.ECC_DRY_RUN; + + const result = spawnSync(process.execPath, [ + runWithFlags, + 'pre:write:doc-file-warning', + hookScript, + 'standard,strict', + ], { + input, + encoding: 'utf8', + env, + cwd: path.resolve(__dirname, '..', '..'), + }); + + assert.strictEqual(result.status, 0); + assert.ok( + !result.stderr.includes('[DryRun]'), + 'Expected no [DryRun] tag in normal execution' + ); + })) passed++; else failed++; + + console.log('\necc.js --dry-run flag parsing:'); + + if (test('--dry-run sets ECC_DRY_RUN env var for child commands', () => { + const eccJs = path.resolve(__dirname, '..', '..', 'scripts', 'ecc.js'); + const result = spawnSync(process.execPath, [eccJs, '--dry-run', '--help'], { + encoding: 'utf8', + env: { ...process.env }, + }); + assert.strictEqual(result.status, 0); + assert.ok(result.stdout.includes('--dry-run'), 'Help text should mention --dry-run'); + })) passed++; else failed++; + + if (test('--dry-run is stripped from args so command routing works', () => { + const eccJs = path.resolve(__dirname, '..', '..', 'scripts', 'ecc.js'); + const result = spawnSync(process.execPath, [eccJs, '--dry-run', 'doctor'], { + encoding: 'utf8', + env: { ...process.env }, + }); + assert.ok( + !result.stderr.includes('Unknown command: --dry-run'), + 'Global --dry-run must not be treated as an unknown command' + ); + })) passed++; else failed++; + + if (test('--dry-run works with implicit install routing', () => { + const eccJs = path.resolve(__dirname, '..', '..', 'scripts', 'ecc.js'); + const result = spawnSync(process.execPath, [eccJs, '--dry-run', '--json', 'typescript'], { + encoding: 'utf8', + env: { ...process.env }, + }); + assert.strictEqual(result.status, 0, `Expected exit 0, got ${result.status}: ${result.stderr}`); + const payload = JSON.parse(result.stdout); + assert.strictEqual(payload.dryRun, true, 'Expected dryRun=true in JSON output'); + assert.deepStrictEqual(payload.plan.legacyLanguages, ['typescript']); + })) passed++; else failed++; + + console.log(`\nResults: ${passed} passed, ${failed} failed`); + process.exit(failed > 0 ? 1 : 0); +} + +runTests();