diff --git a/bin/install.js b/bin/install.js index a97a6717..54e6c83c 100755 --- a/bin/install.js +++ b/bin/install.js @@ -4255,7 +4255,7 @@ function uninstall(isGlobal, runtime = 'claude') { // 4. Remove GSD hooks const hooksDir = path.join(targetDir, 'hooks'); if (fs.existsSync(hooksDir)) { - const gsdHooks = ['gsd-statusline.js', 'gsd-check-update.js', 'gsd-check-update.sh', 'gsd-context-monitor.js', 'gsd-prompt-guard.js', 'gsd-session-state.sh', 'gsd-validate-commit.sh', 'gsd-phase-boundary.sh']; + const gsdHooks = ['gsd-statusline.js', 'gsd-check-update.js', 'gsd-check-update.sh', 'gsd-context-monitor.js', 'gsd-prompt-guard.js', 'gsd-read-guard.js', 'gsd-session-state.sh', 'gsd-validate-commit.sh', 'gsd-phase-boundary.sh']; let hookCount = 0; for (const hook of gsdHooks) { const hookPath = path.join(hooksDir, hook); @@ -4357,7 +4357,7 @@ function uninstall(isGlobal, runtime = 'claude') { settings.hooks[eventName] = settings.hooks[eventName].filter(entry => { if (entry.hooks && Array.isArray(entry.hooks)) { const hasGsdHook = entry.hooks.some(h => - h.command && h.command.includes('gsd-prompt-guard') + h.command && (h.command.includes('gsd-prompt-guard') || h.command.includes('gsd-read-guard')) ); return !hasGsdHook; } @@ -4365,7 +4365,7 @@ function uninstall(isGlobal, runtime = 'claude') { }); if (settings.hooks[eventName].length < before) { settingsModified = true; - console.log(` ${green}✓${reset} Removed prompt injection guard hook from settings`); + console.log(` ${green}✓${reset} Removed GSD pre-tool guard hooks from settings`); } if (settings.hooks[eventName].length === 0) { delete settings.hooks[eventName]; @@ -5381,6 +5381,9 @@ function install(isGlobal, runtime = 'claude') { const promptGuardCommand = isGlobal ? buildHookCommand(targetDir, 'gsd-prompt-guard.js') : 'node ' + dirName + '/hooks/gsd-prompt-guard.js'; + const readGuardCommand = isGlobal + ? buildHookCommand(targetDir, 'gsd-read-guard.js') + : 'node ' + dirName + '/hooks/gsd-read-guard.js'; // Enable experimental agents for Gemini CLI (required for custom sub-agents) if (isGemini) { @@ -5486,6 +5489,27 @@ function install(isGlobal, runtime = 'claude') { console.log(` ${green}✓${reset} Configured prompt injection guard hook`); } + // Configure PreToolUse hook for read-before-edit guidance (#1628) + // Prevents infinite retry loops when non-Claude models attempt to edit + // files without reading them first. Advisory-only — does not block. + const hasReadGuardHook = settings.hooks[preToolEvent].some(entry => + entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-read-guard')) + ); + + if (!hasReadGuardHook) { + settings.hooks[preToolEvent].push({ + matcher: 'Write|Edit', + hooks: [ + { + type: 'command', + command: readGuardCommand, + timeout: 5 + } + ] + }); + console.log(` ${green}✓${reset} Configured read-before-edit guard hook`); + } + // Community hooks — registered on install but opt-in at runtime. // Each hook checks .planning/config.json for hooks.community: true // and exits silently (no-op) if not enabled. This lets users enable diff --git a/hooks/gsd-read-guard.js b/hooks/gsd-read-guard.js new file mode 100644 index 00000000..d1fe3b1e --- /dev/null +++ b/hooks/gsd-read-guard.js @@ -0,0 +1,77 @@ +#!/usr/bin/env node +// gsd-hook-version: {{GSD_VERSION}} +// GSD Read Guard — PreToolUse hook +// Injects advisory guidance when Write/Edit targets an existing file, +// reminding the model to Read the file first. +// +// Background: Non-Claude models (e.g. MiniMax M2.5 on OpenCode) don't +// natively follow the read-before-edit pattern. When they attempt to +// Write/Edit an existing file without reading it, the runtime rejects +// with "You must read file before overwriting it." The model retries +// without reading, creating an infinite loop that burns through usage. +// +// This hook prevents that loop by injecting clear guidance BEFORE the +// tool call reaches the runtime. The model sees the advisory and can +// issue a Read call on the next turn. +// +// Triggers on: Write and Edit tool calls +// Action: Advisory (does not block) — injects read-first guidance +// Only fires when the target file already exists on disk. + +const fs = require('fs'); +const path = require('path'); + +let input = ''; +const stdinTimeout = setTimeout(() => process.exit(0), 3000); +process.stdin.setEncoding('utf8'); +process.stdin.on('data', chunk => input += chunk); +process.stdin.on('end', () => { + clearTimeout(stdinTimeout); + try { + const data = JSON.parse(input); + const toolName = data.tool_name; + + // Only intercept Write and Edit tool calls + if (toolName !== 'Write' && toolName !== 'Edit') { + process.exit(0); + } + + const filePath = data.tool_input?.file_path || ''; + if (!filePath) { + process.exit(0); + } + + // Only inject guidance when the file already exists. + // New files don't need a prior Read — the runtime allows creating them directly. + let fileExists = false; + try { + fs.accessSync(filePath, fs.constants.F_OK); + fileExists = true; + } catch { + // File does not exist — no guidance needed + } + + if (!fileExists) { + process.exit(0); + } + + const fileName = path.basename(filePath); + + // Advisory guidance — does not block the operation + const output = { + hookSpecificOutput: { + hookEventName: 'PreToolUse', + additionalContext: + `READ-BEFORE-EDIT REMINDER: You are about to modify "${fileName}" which already exists. ` + + 'If you have not already used the Read tool to read this file in the current session, ' + + 'you MUST Read it first before editing. The runtime will reject edits to files that ' + + 'have not been read. Use the Read tool on this file path, then retry your edit.', + }, + }; + + process.stdout.write(JSON.stringify(output)); + } catch { + // Silent fail — never block tool execution + process.exit(0); + } +}); diff --git a/scripts/build-hooks.js b/scripts/build-hooks.js index 5c02cbfd..092e442c 100644 --- a/scripts/build-hooks.js +++ b/scripts/build-hooks.js @@ -18,6 +18,7 @@ const HOOKS_TO_COPY = [ 'gsd-check-update.js', 'gsd-context-monitor.js', 'gsd-prompt-guard.js', + 'gsd-read-guard.js', 'gsd-statusline.js', 'gsd-workflow-guard.js' ]; diff --git a/tests/read-guard.test.cjs b/tests/read-guard.test.cjs new file mode 100644 index 00000000..92999d24 --- /dev/null +++ b/tests/read-guard.test.cjs @@ -0,0 +1,224 @@ +/** + * Tests for gsd-read-guard.js PreToolUse hook. + * + * The read guard intercepts Write/Edit tool calls on existing files and injects + * advisory guidance telling the model to Read the file first. This prevents + * infinite retry loops when non-Claude models (e.g. MiniMax M2.5 on OpenCode) + * attempt to edit files without reading them, hitting the runtime's + * "You must read file before overwriting it" error repeatedly. + * + * The hook is advisory-only (does not block) so Claude Code behavior is unaffected. + */ + +process.env.GSD_TEST_MODE = '1'; + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { execFileSync } = require('node:child_process'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-read-guard.js'); + +/** + * Run the read guard hook with a given tool input payload. + * Returns { exitCode, stdout, stderr }. + */ +function runHook(payload) { + const input = JSON.stringify(payload); + try { + const stdout = execFileSync(process.execPath, [HOOK_PATH], { + input, + encoding: 'utf-8', + timeout: 5000, + stdio: ['pipe', 'pipe', 'pipe'], + }); + return { exitCode: 0, stdout: stdout.trim(), stderr: '' }; + } catch (err) { + return { + exitCode: err.status ?? 1, + stdout: (err.stdout || '').toString().trim(), + stderr: (err.stderr || '').toString().trim(), + }; + } +} + +describe('gsd-read-guard hook', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempDir('gsd-read-guard-'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // ─── Core: advisory on Write to existing file ─────────────────────────── + + test('injects read-first guidance when Write targets an existing file', () => { + const filePath = path.join(tmpDir, 'existing.js'); + fs.writeFileSync(filePath, 'console.log("hello");\n'); + + const result = runHook({ + tool_name: 'Write', + tool_input: { file_path: filePath, content: 'console.log("world");\n' }, + }); + + assert.equal(result.exitCode, 0); + assert.ok(result.stdout.length > 0, 'should produce output'); + + const output = JSON.parse(result.stdout); + assert.ok(output.hookSpecificOutput, 'should have hookSpecificOutput'); + assert.ok(output.hookSpecificOutput.additionalContext, 'should have additionalContext'); + assert.ok( + output.hookSpecificOutput.additionalContext.includes('Read'), + 'guidance should mention Read tool' + ); + }); + + test('injects read-first guidance when Edit targets an existing file', () => { + const filePath = path.join(tmpDir, 'existing.js'); + fs.writeFileSync(filePath, 'const x = 1;\n'); + + const result = runHook({ + tool_name: 'Edit', + tool_input: { file_path: filePath, old_string: 'const x = 1;', new_string: 'const x = 2;' }, + }); + + assert.equal(result.exitCode, 0); + assert.ok(result.stdout.length > 0, 'should produce output'); + + const output = JSON.parse(result.stdout); + assert.ok(output.hookSpecificOutput.additionalContext.includes('Read')); + }); + + // ─── No-op cases: should NOT inject guidance ──────────────────────────── + + test('does nothing for Write to a new file (file does not exist)', () => { + const filePath = path.join(tmpDir, 'brand-new.js'); + // File does NOT exist + + const result = runHook({ + tool_name: 'Write', + tool_input: { file_path: filePath, content: 'new content' }, + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, '', 'should produce no output for new files'); + }); + + test('does nothing for non-Write/Edit tools', () => { + const result = runHook({ + tool_name: 'Bash', + tool_input: { command: 'echo hello' }, + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, ''); + }); + + test('does nothing for Read tool', () => { + const filePath = path.join(tmpDir, 'existing.js'); + fs.writeFileSync(filePath, 'content'); + + const result = runHook({ + tool_name: 'Read', + tool_input: { file_path: filePath }, + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, ''); + }); + + // ─── Error resilience ────────────────────────────────────────────────── + + test('exits cleanly on invalid JSON input', () => { + try { + const stdout = execFileSync(process.execPath, [HOOK_PATH], { + input: 'not json', + encoding: 'utf-8', + timeout: 5000, + stdio: ['pipe', 'pipe', 'pipe'], + }); + // Should exit 0 silently + assert.equal(stdout.trim(), ''); + } catch (err) { + assert.equal(err.status, 0, 'should exit 0 on parse error'); + } + }); + + test('exits cleanly when tool_input is missing', () => { + const result = runHook({ tool_name: 'Write' }); + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, ''); + }); + + // ─── Guidance content quality ────────────────────────────────────────── + + test('guidance message includes the filename', () => { + const filePath = path.join(tmpDir, 'myfile.ts'); + fs.writeFileSync(filePath, 'export const foo = 1;\n'); + + const result = runHook({ + tool_name: 'Write', + tool_input: { file_path: filePath, content: 'export const foo = 2;\n' }, + }); + + const output = JSON.parse(result.stdout); + assert.ok( + output.hookSpecificOutput.additionalContext.includes('myfile.ts'), + 'guidance should include the filename being edited' + ); + }); + + test('guidance message instructs to use Read tool before editing', () => { + const filePath = path.join(tmpDir, 'target.py'); + fs.writeFileSync(filePath, 'x = 1\n'); + + const result = runHook({ + tool_name: 'Edit', + tool_input: { file_path: filePath, old_string: 'x = 1', new_string: 'x = 2' }, + }); + + const output = JSON.parse(result.stdout); + const ctx = output.hookSpecificOutput.additionalContext; + assert.ok(ctx.includes('Read'), 'must mention Read tool'); + assert.ok( + ctx.includes('before') || ctx.includes('first'), + 'must indicate Read should come before the edit' + ); + }); + + // ─── Build / install integration ─────────────────────────────────────── + + test('hook is registered in build-hooks.js HOOKS_TO_COPY', () => { + const buildHooksPath = path.join(__dirname, '..', 'scripts', 'build-hooks.js'); + const content = fs.readFileSync(buildHooksPath, 'utf8'); + assert.ok( + content.includes('gsd-read-guard.js'), + 'gsd-read-guard.js must be in HOOKS_TO_COPY so it ships in hooks/dist/' + ); + }); + + test('hook is registered in install.js uninstall hook list', () => { + const installPath = path.join(__dirname, '..', 'bin', 'install.js'); + const content = fs.readFileSync(installPath, 'utf8'); + assert.ok( + content.includes("'gsd-read-guard.js'"), + 'gsd-read-guard.js must be in the uninstall gsdHooks list' + ); + }); + + test('exits cleanly when tool_input.file_path is non-string', () => { + const result = runHook({ + tool_name: 'Write', + tool_input: { file_path: 12345, content: 'data' }, + }); + // file_path is a number — || '' yields '' — hook exits silently + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, ''); + }); +});