From 992688a674943672d2b3fd596a977eead94e8df8 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 13 Feb 2026 03:20:41 -0800 Subject: [PATCH] fix: add cwd to prettier hook, consistent process.exit(0), and stdout pass-through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - post-edit-format.js: add cwd based on file directory so npx resolves correct local prettier binary - post-edit-typecheck.js, post-edit-format.js: replace console.log(data) with process.stdout.write(data) to avoid trailing newline corruption - Add process.exit(0) to 4 hooks for consistent termination (check-console-log, post-edit-console-warn, post-edit-format, post-edit-typecheck) - run-all.js: switch from execSync to spawnSync so stderr is visible on the success path (hook warnings were silently discarded) - Add 21 tests: cwd verification, process.exit(0) checks, exact stdout pass-through, extension edge cases, exclusion pattern matching, threshold boundary values (630 → 651) --- scripts/hooks/check-console-log.js | 1 + scripts/hooks/post-edit-console-warn.js | 1 + scripts/hooks/post-edit-format.js | 5 +- scripts/hooks/post-edit-typecheck.js | 6 +- tests/hooks/hooks.test.js | 157 +++++++++++++++++++++++- tests/hooks/suggest-compact.test.js | 73 +++++++++++ tests/run-all.js | 39 +++--- 7 files changed, 255 insertions(+), 27 deletions(-) diff --git a/scripts/hooks/check-console-log.js b/scripts/hooks/check-console-log.js index 6a08326..3658dde 100755 --- a/scripts/hooks/check-console-log.js +++ b/scripts/hooks/check-console-log.js @@ -66,4 +66,5 @@ process.stdin.on('end', () => { // Always output the original data process.stdout.write(data); + process.exit(0); }); diff --git a/scripts/hooks/post-edit-console-warn.js b/scripts/hooks/post-edit-console-warn.js index a5d71e9..76f1ee2 100644 --- a/scripts/hooks/post-edit-console-warn.js +++ b/scripts/hooks/post-edit-console-warn.js @@ -49,4 +49,5 @@ process.stdin.on('end', () => { } process.stdout.write(data); + process.exit(0); }); diff --git a/scripts/hooks/post-edit-format.js b/scripts/hooks/post-edit-format.js index 827fbd2..f3651b5 100644 --- a/scripts/hooks/post-edit-format.js +++ b/scripts/hooks/post-edit-format.js @@ -9,6 +9,7 @@ */ const { execFileSync } = require('child_process'); +const path = require('path'); const MAX_STDIN = 1024 * 1024; // 1MB limit let data = ''; @@ -30,6 +31,7 @@ process.stdin.on('end', () => { // Use npx.cmd on Windows to avoid shell: true which enables command injection const npxBin = process.platform === 'win32' ? 'npx.cmd' : 'npx'; execFileSync(npxBin, ['prettier', '--write', filePath], { + cwd: path.dirname(path.resolve(filePath)), stdio: ['pipe', 'pipe', 'pipe'], timeout: 15000 }); @@ -41,5 +43,6 @@ process.stdin.on('end', () => { // Invalid input — pass through } - console.log(data); + process.stdout.write(data); + process.exit(0); }); diff --git a/scripts/hooks/post-edit-typecheck.js b/scripts/hooks/post-edit-typecheck.js index 31f6c06..981dbeb 100644 --- a/scripts/hooks/post-edit-typecheck.js +++ b/scripts/hooks/post-edit-typecheck.js @@ -31,7 +31,8 @@ process.stdin.on("end", () => { if (filePath && /\.(ts|tsx)$/.test(filePath)) { const resolvedPath = path.resolve(filePath); if (!fs.existsSync(resolvedPath)) { - console.log(data); + process.stdout.write(data); + process.exit(0); return; } // Find nearest tsconfig.json by walking up (max 20 levels to prevent infinite loop) @@ -90,5 +91,6 @@ process.stdin.on("end", () => { // Invalid input — pass through } - console.log(data); + process.stdout.write(data); + process.exit(0); }); diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 9045bc6..ca51982 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -1945,7 +1945,7 @@ async function runTests() { const malformedJson = '{"tool_input": {"file_path": "/test.ts"'; const result = await runScript(path.join(scriptsDir, 'post-edit-format.js'), malformedJson); assert.strictEqual(result.code, 0); - // Should pass through the malformed data (console.log adds \n) + // Should pass through the malformed data unchanged assert.ok(result.stdout.includes(malformedJson), 'Should pass through malformed JSON'); })) passed++; else failed++; @@ -2079,6 +2079,161 @@ async function runTests() { assert.strictEqual(result.code, 0, 'Should exit 0 with empty stdin'); })) passed++; else failed++; + // ── Round 29: post-edit-format.js cwd fix and process.exit(0) consistency ── + console.log('\nRound 29: post-edit-format.js (cwd and exit):'); + + if (await asyncTest('source uses cwd based on file directory for npx', async () => { + const formatSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-format.js'), 'utf8'); + assert.ok(formatSource.includes('cwd:'), 'Should set cwd option for execFileSync'); + assert.ok(formatSource.includes('path.dirname'), 'cwd should use path.dirname of the file'); + assert.ok(formatSource.includes('path.resolve'), 'cwd should resolve the file path first'); + })) passed++; else failed++; + + if (await asyncTest('source calls process.exit(0) after writing output', async () => { + const formatSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-format.js'), 'utf8'); + assert.ok(formatSource.includes('process.exit(0)'), 'Should call process.exit(0) for clean termination'); + })) passed++; else failed++; + + if (await asyncTest('uses process.stdout.write instead of console.log for pass-through', async () => { + const formatSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-format.js'), 'utf8'); + assert.ok(formatSource.includes('process.stdout.write(data)'), 'Should use process.stdout.write to avoid trailing newline'); + // Verify no console.log(data) for pass-through (console.error for warnings is OK) + const lines = formatSource.split('\n'); + const passThrough = lines.filter(l => /console\.log\(data\)/.test(l)); + assert.strictEqual(passThrough.length, 0, 'Should not use console.log(data) for pass-through'); + })) passed++; else failed++; + + console.log('\nRound 29: post-edit-typecheck.js (exit and pass-through):'); + + if (await asyncTest('source calls process.exit(0) after writing output', async () => { + const tcSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-typecheck.js'), 'utf8'); + assert.ok(tcSource.includes('process.exit(0)'), 'Should call process.exit(0) for clean termination'); + })) passed++; else failed++; + + if (await asyncTest('uses process.stdout.write instead of console.log for pass-through', async () => { + const tcSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-typecheck.js'), 'utf8'); + assert.ok(tcSource.includes('process.stdout.write(data)'), 'Should use process.stdout.write'); + const lines = tcSource.split('\n'); + const passThrough = lines.filter(l => /console\.log\(data\)/.test(l)); + assert.strictEqual(passThrough.length, 0, 'Should not use console.log(data) for pass-through'); + })) passed++; else failed++; + + if (await asyncTest('exact stdout pass-through without trailing newline (typecheck)', async () => { + const stdinJson = JSON.stringify({ tool_input: { file_path: '/nonexistent/file.py' } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-typecheck.js'), stdinJson); + assert.strictEqual(result.code, 0); + assert.strictEqual(result.stdout, stdinJson, 'stdout should exactly match stdin (no trailing newline)'); + })) passed++; else failed++; + + if (await asyncTest('exact stdout pass-through without trailing newline (format)', async () => { + const stdinJson = JSON.stringify({ tool_input: { file_path: '/nonexistent/file.py' } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-format.js'), stdinJson); + assert.strictEqual(result.code, 0); + assert.strictEqual(result.stdout, stdinJson, 'stdout should exactly match stdin (no trailing newline)'); + })) passed++; else failed++; + + console.log('\nRound 29: post-edit-console-warn.js (extension and exit):'); + + if (await asyncTest('source calls process.exit(0) after writing output', async () => { + const cwSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-console-warn.js'), 'utf8'); + assert.ok(cwSource.includes('process.exit(0)'), 'Should call process.exit(0)'); + })) passed++; else failed++; + + if (await asyncTest('does NOT match .mts or .mjs extensions', async () => { + const stdinMts = JSON.stringify({ tool_input: { file_path: '/some/file.mts' } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-console-warn.js'), stdinMts); + assert.strictEqual(result.code, 0); + // .mts is not in the regex /\.(ts|tsx|js|jsx)$/, so no console.log scan + assert.strictEqual(result.stdout, stdinMts, 'Should pass through .mts without scanning'); + assert.ok(!result.stderr.includes('console.log'), 'Should NOT scan .mts files for console.log'); + })) passed++; else failed++; + + if (await asyncTest('does NOT match uppercase .TS extension', async () => { + const stdinTS = JSON.stringify({ tool_input: { file_path: '/some/file.TS' } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-console-warn.js'), stdinTS); + assert.strictEqual(result.code, 0); + assert.strictEqual(result.stdout, stdinTS, 'Should pass through .TS without scanning'); + assert.ok(!result.stderr.includes('console.log'), 'Should NOT scan .TS (uppercase) files'); + })) passed++; else failed++; + + if (await asyncTest('detects console.log in commented-out code', async () => { + const testDir = createTestDir(); + const testFile = path.join(testDir, 'commented.js'); + fs.writeFileSync(testFile, '// console.log("debug")\nconst x = 1;\n'); + const stdinJson = JSON.stringify({ tool_input: { file_path: testFile } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-console-warn.js'), stdinJson); + assert.strictEqual(result.code, 0); + // The regex /console\.log/ matches even in comments — this is intentional + assert.ok(result.stderr.includes('console.log'), 'Should detect console.log even in comments'); + cleanupTestDir(testDir); + })) passed++; else failed++; + + console.log('\nRound 29: check-console-log.js (exclusion patterns and exit):'); + + if (await asyncTest('source calls process.exit(0) after writing output', async () => { + const clSource = fs.readFileSync(path.join(scriptsDir, 'check-console-log.js'), 'utf8'); + // Should have at least 2 process.exit(0) calls (early return + end) + const exitCalls = clSource.match(/process\.exit\(0\)/g) || []; + assert.ok(exitCalls.length >= 2, `Should have at least 2 process.exit(0) calls, found ${exitCalls.length}`); + })) passed++; else failed++; + + if (await asyncTest('EXCLUDED_PATTERNS correctly excludes test files', async () => { + // Test the patterns directly by reading the source and evaluating the regex + const source = fs.readFileSync(path.join(scriptsDir, 'check-console-log.js'), 'utf8'); + // Verify the 6 exclusion patterns exist in the source (as regex literals with escapes) + const expectedSubstrings = ['test', 'spec', 'config', 'scripts', '__tests__', '__mocks__']; + for (const substr of expectedSubstrings) { + assert.ok(source.includes(substr), `Should include pattern containing "${substr}"`); + } + // Verify the array name exists + assert.ok(source.includes('EXCLUDED_PATTERNS'), 'Should have EXCLUDED_PATTERNS array'); + })) passed++; else failed++; + + if (await asyncTest('exclusion patterns match expected file paths', async () => { + // Recreate the EXCLUDED_PATTERNS from the source and test them + const EXCLUDED_PATTERNS = [ + /\.test\.[jt]sx?$/, + /\.spec\.[jt]sx?$/, + /\.config\.[jt]s$/, + /scripts\//, + /__tests__\//, + /__mocks__\//, + ]; + // These SHOULD be excluded + const excluded = [ + 'src/utils.test.ts', 'src/utils.test.js', 'src/utils.test.tsx', 'src/utils.test.jsx', + 'src/utils.spec.ts', 'src/utils.spec.js', + 'src/utils.config.ts', 'src/utils.config.js', + 'scripts/hooks/session-end.js', + '__tests__/utils.ts', + '__mocks__/api.ts', + ]; + for (const f of excluded) { + const matches = EXCLUDED_PATTERNS.some(p => p.test(f)); + assert.ok(matches, `Expected "${f}" to be excluded but it was not`); + } + // These should NOT be excluded + const notExcluded = [ + 'src/utils.ts', 'src/main.tsx', 'src/app.js', + 'src/test.component.ts', // "test" in name but not .test. pattern + 'src/config.ts', // "config" in name but not .config. pattern + ]; + for (const f of notExcluded) { + const matches = EXCLUDED_PATTERNS.some(p => p.test(f)); + assert.ok(!matches, `Expected "${f}" to NOT be excluded but it was`); + } + })) passed++; else failed++; + + console.log('\nRound 29: run-all.js test runner improvements:'); + + if (await asyncTest('test runner uses spawnSync to capture stderr on success', async () => { + const runAllSource = fs.readFileSync(path.join(__dirname, '..', 'run-all.js'), 'utf8'); + assert.ok(runAllSource.includes('spawnSync'), 'Should use spawnSync instead of execSync'); + assert.ok(!runAllSource.includes('execSync'), 'Should not use execSync'); + // Verify it shows stderr + assert.ok(runAllSource.includes('stderr'), 'Should handle stderr output'); + })) passed++; else failed++; + // Summary console.log('\n=== Test Results ==='); console.log(`Passed: ${passed}`); diff --git a/tests/hooks/suggest-compact.test.js b/tests/hooks/suggest-compact.test.js index 4dc3a6b..26733bb 100644 --- a/tests/hooks/suggest-compact.test.js +++ b/tests/hooks/suggest-compact.test.js @@ -245,6 +245,79 @@ function runTests() { cleanupCounter(); })) passed++; else failed++; + // ── Round 29: threshold boundary values ── + console.log('\nThreshold boundary values:'); + + if (test('rejects COMPACT_THRESHOLD=0 (falls back to 50)', () => { + cleanupCounter(); + fs.writeFileSync(counterFile, '49'); + const result = runCompact({ CLAUDE_SESSION_ID: testSession, COMPACT_THRESHOLD: '0' }); + // 0 is invalid (must be > 0), falls back to 50, count becomes 50 → should suggest + assert.ok( + result.stderr.includes('50 tool calls reached'), + `Should fallback to 50 for threshold=0. Got stderr: ${result.stderr}` + ); + cleanupCounter(); + })) passed++; else failed++; + + if (test('accepts COMPACT_THRESHOLD=10000 (boundary max)', () => { + cleanupCounter(); + fs.writeFileSync(counterFile, '9999'); + const result = runCompact({ CLAUDE_SESSION_ID: testSession, COMPACT_THRESHOLD: '10000' }); + // count becomes 10000, threshold=10000 → should suggest + assert.ok( + result.stderr.includes('10000 tool calls reached'), + `Should accept threshold=10000. Got stderr: ${result.stderr}` + ); + cleanupCounter(); + })) passed++; else failed++; + + if (test('rejects COMPACT_THRESHOLD=10001 (falls back to 50)', () => { + cleanupCounter(); + fs.writeFileSync(counterFile, '49'); + const result = runCompact({ CLAUDE_SESSION_ID: testSession, COMPACT_THRESHOLD: '10001' }); + // 10001 > 10000, invalid, falls back to 50, count becomes 50 → should suggest + assert.ok( + result.stderr.includes('50 tool calls reached'), + `Should fallback to 50 for threshold=10001. Got stderr: ${result.stderr}` + ); + cleanupCounter(); + })) passed++; else failed++; + + if (test('rejects float COMPACT_THRESHOLD (e.g. 3.5)', () => { + cleanupCounter(); + fs.writeFileSync(counterFile, '49'); + const result = runCompact({ CLAUDE_SESSION_ID: testSession, COMPACT_THRESHOLD: '3.5' }); + // parseInt('3.5') = 3, which is valid (> 0 && <= 10000) + // count becomes 50, threshold=3, 50-3=47, 47%25≠0 and 50≠3 → no suggestion + assert.strictEqual(result.code, 0); + // No suggestion expected (50 !== 3, and (50-3) % 25 !== 0) + assert.ok( + !result.stderr.includes('StrategicCompact'), + 'Float threshold should be parseInt-ed to 3, no suggestion at count=50' + ); + cleanupCounter(); + })) passed++; else failed++; + + if (test('counter value at exact boundary 1000000 is valid', () => { + cleanupCounter(); + fs.writeFileSync(counterFile, '999999'); + const result = runCompact({ CLAUDE_SESSION_ID: testSession, COMPACT_THRESHOLD: '3' }); + // 999999 is valid (> 0, <= 1000000), count becomes 1000000 + const count = parseInt(fs.readFileSync(counterFile, 'utf8').trim(), 10); + assert.strictEqual(count, 1000000, 'Counter at 1000000 boundary should be valid'); + cleanupCounter(); + })) passed++; else failed++; + + if (test('counter value at 1000001 is clamped (reset to 1)', () => { + cleanupCounter(); + fs.writeFileSync(counterFile, '1000001'); + const result = runCompact({ CLAUDE_SESSION_ID: testSession }); + const count = parseInt(fs.readFileSync(counterFile, 'utf8').trim(), 10); + assert.strictEqual(count, 1, 'Counter > 1000000 should be reset to 1'); + cleanupCounter(); + })) passed++; else failed++; + // Summary console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); diff --git a/tests/run-all.js b/tests/run-all.js index 4be7c23..b161ad4 100644 --- a/tests/run-all.js +++ b/tests/run-all.js @@ -5,7 +5,7 @@ * Usage: node tests/run-all.js */ -const { execSync } = require('child_process'); +const { spawnSync } = require('child_process'); const path = require('path'); const fs = require('fs'); @@ -46,32 +46,25 @@ for (const testFile of testFiles) { console.log(`\n━━━ Running ${testFile} ━━━`); - try { - const output = execSync(`node "${testPath}"`, { - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'] - }); - console.log(output); + const result = spawnSync('node', [testPath], { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'] + }); - // Parse results from output - const passedMatch = output.match(/Passed:\s*(\d+)/); - const failedMatch = output.match(/Failed:\s*(\d+)/); + const stdout = result.stdout || ''; + const stderr = result.stderr || ''; - if (passedMatch) totalPassed += parseInt(passedMatch[1], 10); - if (failedMatch) totalFailed += parseInt(failedMatch[1], 10); + // Show both stdout and stderr so hook warnings are visible + if (stdout) console.log(stdout); + if (stderr) console.log(stderr); - } catch (err) { - console.log(err.stdout || ''); - console.log(err.stderr || ''); + // Parse results from combined output + const combined = stdout + stderr; + const passedMatch = combined.match(/Passed:\s*(\d+)/); + const failedMatch = combined.match(/Failed:\s*(\d+)/); - // Parse results even on failure - const output = (err.stdout || '') + (err.stderr || ''); - const passedMatch = output.match(/Passed:\s*(\d+)/); - const failedMatch = output.match(/Failed:\s*(\d+)/); - - if (passedMatch) totalPassed += parseInt(passedMatch[1], 10); - if (failedMatch) totalFailed += parseInt(failedMatch[1], 10); - } + if (passedMatch) totalPassed += parseInt(passedMatch[1], 10); + if (failedMatch) totalFailed += parseInt(failedMatch[1], 10); } totalTests = totalPassed + totalFailed;