From 6f95dbe7ba2629a756ba3e7b644a9c1afa88f0ad Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 13 Feb 2026 01:18:07 -0800 Subject: [PATCH] fix: grepFile global regex lastIndex bug, add 12 tests Fix grepFile() silently skipping matches when called with /g flag regex. The global flag makes .test() stateful, causing alternating match/miss on consecutive matching lines. Strip g flag since per-line testing doesn't need global state. Add first-ever tests for evaluate-session.js (5 tests: short session, long session, missing transcript, malformed stdin, env var fallback) and suggest-compact.js (5 tests: counter increment, threshold trigger, periodic suggestions, below-threshold silence, invalid threshold). --- scripts/lib/utils.js | 10 ++- tests/hooks/hooks.test.js | 152 ++++++++++++++++++++++++++++++++++++++ tests/lib/utils.test.js | 31 ++++++++ 3 files changed, 192 insertions(+), 1 deletion(-) diff --git a/scripts/lib/utils.js b/scripts/lib/utils.js index ec145213..c9d08418 100644 --- a/scripts/lib/utils.js +++ b/scripts/lib/utils.js @@ -454,7 +454,15 @@ function grepFile(filePath, pattern) { let regex; try { - regex = pattern instanceof RegExp ? pattern : new RegExp(pattern); + if (pattern instanceof RegExp) { + // Always create a new RegExp without the 'g' flag to prevent lastIndex + // state issues when using .test() in a loop (g flag makes .test() stateful, + // causing alternating match/miss on consecutive matching lines) + const flags = pattern.flags.replace('g', ''); + regex = new RegExp(pattern.source, flags); + } else { + regex = new RegExp(pattern); + } } catch { return []; // Invalid regex pattern } diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 5933bff8..cddcf117 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -987,6 +987,158 @@ async function runTests() { ); })) passed++; else failed++; + // ─── evaluate-session.js tests ─── + console.log('\nevaluate-session.js:'); + + if (await asyncTest('skips when no transcript_path in stdin', async () => { + const result = await runScript(path.join(scriptsDir, 'evaluate-session.js'), '{}'); + assert.strictEqual(result.code, 0, 'Should exit 0 (non-blocking)'); + })) passed++; else failed++; + + if (await asyncTest('skips when transcript file does not exist', async () => { + const stdinJson = JSON.stringify({ transcript_path: '/tmp/nonexistent-transcript-12345.jsonl' }); + const result = await runScript(path.join(scriptsDir, 'evaluate-session.js'), stdinJson); + assert.strictEqual(result.code, 0, 'Should exit 0 when file missing'); + })) passed++; else failed++; + + if (await asyncTest('skips short sessions (< 10 user messages)', async () => { + const testDir = createTestDir(); + const transcriptPath = path.join(testDir, 'short.jsonl'); + // Only 3 user messages — below the default threshold of 10 + const lines = [ + '{"type":"user","content":"msg1"}', + '{"type":"user","content":"msg2"}', + '{"type":"user","content":"msg3"}', + ]; + fs.writeFileSync(transcriptPath, lines.join('\n')); + const stdinJson = JSON.stringify({ transcript_path: transcriptPath }); + const result = await runScript(path.join(scriptsDir, 'evaluate-session.js'), stdinJson); + assert.strictEqual(result.code, 0); + assert.ok(result.stderr.includes('too short'), 'Should log "too short" message'); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (await asyncTest('evaluates long sessions (>= 10 user messages)', async () => { + const testDir = createTestDir(); + const transcriptPath = path.join(testDir, 'long.jsonl'); + // 12 user messages — above the default threshold + const lines = []; + for (let i = 0; i < 12; i++) { + lines.push(`{"type":"user","content":"message ${i}"}`); + } + fs.writeFileSync(transcriptPath, lines.join('\n')); + const stdinJson = JSON.stringify({ transcript_path: transcriptPath }); + const result = await runScript(path.join(scriptsDir, 'evaluate-session.js'), stdinJson); + assert.strictEqual(result.code, 0); + assert.ok(result.stderr.includes('12 messages'), 'Should report message count'); + assert.ok(result.stderr.includes('evaluate'), 'Should signal evaluation'); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (await asyncTest('handles malformed stdin JSON (falls back to env var)', async () => { + const result = await runScript( + path.join(scriptsDir, 'evaluate-session.js'), + 'not json at all', + { CLAUDE_TRANSCRIPT_PATH: '' } + ); + // No valid transcript path from either source → exit 0 + assert.strictEqual(result.code, 0); + })) passed++; else failed++; + + // ─── suggest-compact.js tests ─── + console.log('\nsuggest-compact.js:'); + + if (await asyncTest('increments tool counter on each invocation', async () => { + const sessionId = `test-counter-${Date.now()}`; + const counterFile = path.join(os.tmpdir(), `claude-tool-count-${sessionId}`); + try { + // First invocation → count = 1 + await runScript(path.join(scriptsDir, 'suggest-compact.js'), '', { + CLAUDE_SESSION_ID: sessionId + }); + let val = parseInt(fs.readFileSync(counterFile, 'utf8').trim(), 10); + assert.strictEqual(val, 1, 'First call should write count 1'); + + // Second invocation → count = 2 + await runScript(path.join(scriptsDir, 'suggest-compact.js'), '', { + CLAUDE_SESSION_ID: sessionId + }); + val = parseInt(fs.readFileSync(counterFile, 'utf8').trim(), 10); + assert.strictEqual(val, 2, 'Second call should write count 2'); + } finally { + try { fs.unlinkSync(counterFile); } catch {} + } + })) passed++; else failed++; + + if (await asyncTest('suggests compact at exact threshold', async () => { + const sessionId = `test-threshold-${Date.now()}`; + const counterFile = path.join(os.tmpdir(), `claude-tool-count-${sessionId}`); + try { + // Pre-seed counter at threshold - 1 so next call hits threshold + fs.writeFileSync(counterFile, '4'); + const result = await runScript(path.join(scriptsDir, 'suggest-compact.js'), '', { + CLAUDE_SESSION_ID: sessionId, + COMPACT_THRESHOLD: '5' + }); + assert.strictEqual(result.code, 0); + assert.ok(result.stderr.includes('5 tool calls reached'), 'Should suggest compact at threshold'); + } finally { + try { fs.unlinkSync(counterFile); } catch {} + } + })) passed++; else failed++; + + if (await asyncTest('suggests at periodic intervals after threshold', async () => { + const sessionId = `test-periodic-${Date.now()}`; + const counterFile = path.join(os.tmpdir(), `claude-tool-count-${sessionId}`); + try { + // Pre-seed at 74 so next call = 75 (threshold 5 + 70, 70 % 25 === 20, not a hit) + // Actually: count > threshold && count % 25 === 0 → need count = 75 + fs.writeFileSync(counterFile, '74'); + const result = await runScript(path.join(scriptsDir, 'suggest-compact.js'), '', { + CLAUDE_SESSION_ID: sessionId, + COMPACT_THRESHOLD: '5' + }); + assert.strictEqual(result.code, 0); + assert.ok(result.stderr.includes('75 tool calls'), 'Should suggest at multiples of 25'); + } finally { + try { fs.unlinkSync(counterFile); } catch {} + } + })) passed++; else failed++; + + if (await asyncTest('does not suggest below threshold', async () => { + const sessionId = `test-below-${Date.now()}`; + const counterFile = path.join(os.tmpdir(), `claude-tool-count-${sessionId}`); + try { + fs.writeFileSync(counterFile, '2'); + const result = await runScript(path.join(scriptsDir, 'suggest-compact.js'), '', { + CLAUDE_SESSION_ID: sessionId, + COMPACT_THRESHOLD: '50' + }); + assert.strictEqual(result.code, 0); + assert.ok(!result.stderr.includes('tool calls reached'), 'Should not suggest below threshold'); + assert.ok(!result.stderr.includes('checkpoint'), 'Should not suggest checkpoint'); + } finally { + try { fs.unlinkSync(counterFile); } catch {} + } + })) passed++; else failed++; + + if (await asyncTest('handles invalid COMPACT_THRESHOLD (falls back to 50)', async () => { + const sessionId = `test-invalid-thresh-${Date.now()}`; + const counterFile = path.join(os.tmpdir(), `claude-tool-count-${sessionId}`); + try { + // Pre-seed at 49 so next call = 50 (the fallback default) + fs.writeFileSync(counterFile, '49'); + const result = await runScript(path.join(scriptsDir, 'suggest-compact.js'), '', { + CLAUDE_SESSION_ID: sessionId, + COMPACT_THRESHOLD: 'not-a-number' + }); + assert.strictEqual(result.code, 0); + assert.ok(result.stderr.includes('50 tool calls reached'), 'Should use default threshold of 50'); + } finally { + try { fs.unlinkSync(counterFile); } catch {} + } + })) passed++; else failed++; + // Summary console.log('\n=== Test Results ==='); console.log(`Passed: ${passed}`); diff --git a/tests/lib/utils.test.js b/tests/lib/utils.test.js index 5f636c20..531037c2 100644 --- a/tests/lib/utils.test.js +++ b/tests/lib/utils.test.js @@ -684,6 +684,37 @@ function runTests() { assert.deepStrictEqual(parsed, { a: { b: 1 }, c: [1, 2] }); })) passed++; else failed++; + // grepFile with global regex (regression: g flag causes alternating matches) + console.log('\ngrepFile (global regex fix):'); + + if (test('grepFile with /g flag finds ALL matching lines (not alternating)', () => { + const testFile = path.join(utils.getTempDir(), `utils-test-grep-g-${Date.now()}.txt`); + try { + // 4 consecutive lines matching the same pattern + utils.writeFile(testFile, 'match-line\nmatch-line\nmatch-line\nmatch-line'); + // Bug: without fix, /match/g would only find lines 1 and 3 (alternating) + const matches = utils.grepFile(testFile, /match/g); + assert.strictEqual(matches.length, 4, `Should find all 4 lines, found ${matches.length}`); + assert.strictEqual(matches[0].lineNumber, 1); + assert.strictEqual(matches[1].lineNumber, 2); + assert.strictEqual(matches[2].lineNumber, 3); + assert.strictEqual(matches[3].lineNumber, 4); + } finally { + fs.unlinkSync(testFile); + } + })) passed++; else failed++; + + if (test('grepFile preserves regex flags other than g (e.g. case-insensitive)', () => { + const testFile = path.join(utils.getTempDir(), `utils-test-grep-flags-${Date.now()}.txt`); + try { + utils.writeFile(testFile, 'FOO\nfoo\nFoO\nbar'); + const matches = utils.grepFile(testFile, /foo/gi); + assert.strictEqual(matches.length, 3, `Should find 3 case-insensitive matches, found ${matches.length}`); + } finally { + fs.unlinkSync(testFile); + } + })) passed++; else failed++; + // Summary console.log('\n=== Test Results ==='); console.log(`Passed: ${passed}`);