From 3f651b7c3c16d400101ef08220ae7d0b1d2784be Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 13 Feb 2026 01:28:59 -0800 Subject: [PATCH] fix: typecheck hook false positives, add 11 session-manager tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix post-edit-typecheck.js error filtering: use relative/absolute path matching instead of basename, preventing false positives when multiple files share the same name (e.g., src/utils.ts vs tests/utils.ts) - Add writeSessionContent tests (create, overwrite, invalid path) - Add appendSessionContent test (append to existing file) - Add deleteSession tests (delete existing, non-existent) - Add sessionExists tests (file, non-existent, directory) - Add getSessionStats empty content edge case - Add post-edit-typecheck stdout passthrough test - Total: 391 → 402 tests, all passing --- scripts/hooks/post-edit-typecheck.js | 18 +++-- tests/hooks/hooks.test.js | 12 +++ tests/lib/session-manager.test.js | 111 +++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 5 deletions(-) diff --git a/scripts/hooks/post-edit-typecheck.js b/scripts/hooks/post-edit-typecheck.js index 5116dd99..b5cfb6eb 100644 --- a/scripts/hooks/post-edit-typecheck.js +++ b/scripts/hooks/post-edit-typecheck.js @@ -59,13 +59,21 @@ process.stdin.on("end", () => { } catch (err) { // tsc exits non-zero when there are errors — filter to edited file const output = (err.stdout || "") + (err.stderr || ""); + // Compute paths that uniquely identify the edited file. + // tsc output uses paths relative to its cwd (the tsconfig dir), + // so check for the relative path, absolute path, and original path. + // Avoid bare basename matching — it causes false positives when + // multiple files share the same name (e.g., src/utils.ts vs tests/utils.ts). + const relPath = path.relative(dir, resolvedPath); + const candidates = new Set([filePath, resolvedPath, relPath]); const relevantLines = output .split("\n") - .filter( - (line) => - line.includes(filePath) || - line.includes(path.basename(filePath)), - ) + .filter((line) => { + for (const candidate of candidates) { + if (line.includes(candidate)) return true; + } + return false; + }) .slice(0, 10); if (relevantLines.length > 0) { diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index cddcf117..d051abe9 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -639,6 +639,18 @@ async function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + if (await asyncTest('passes through stdin data on stdout (post-edit-typecheck)', async () => { + const testDir = createTestDir(); + const testFile = path.join(testDir, 'test.ts'); + fs.writeFileSync(testFile, 'const x: number = 1;'); + + const stdinJson = JSON.stringify({ tool_input: { file_path: testFile } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-typecheck.js'), stdinJson); + assert.strictEqual(result.code, 0); + assert.ok(result.stdout.includes('tool_input'), 'Should pass through stdin data on stdout'); + cleanupTestDir(testDir); + })) passed++; else failed++; + // session-end.js extractSessionSummary tests console.log('\nsession-end.js (extractSessionSummary):'); diff --git a/tests/lib/session-manager.test.js b/tests/lib/session-manager.test.js index 831c074e..07072492 100644 --- a/tests/lib/session-manager.test.js +++ b/tests/lib/session-manager.test.js @@ -621,6 +621,117 @@ src/main.ts assert.ok(!isNaN(result.datetime.getTime()), 'datetime should be valid'); })) passed++; else failed++; + // writeSessionContent tests + console.log('\nwriteSessionContent:'); + + if (test('creates new session file', () => { + const dir = createTempSessionDir(); + try { + const sessionPath = path.join(dir, 'write-test.tmp'); + const result = sessionManager.writeSessionContent(sessionPath, '# Test Session\n'); + assert.strictEqual(result, true, 'Should return true on success'); + assert.ok(fs.existsSync(sessionPath), 'File should exist'); + assert.strictEqual(fs.readFileSync(sessionPath, 'utf8'), '# Test Session\n'); + } finally { + cleanup(dir); + } + })) passed++; else failed++; + + if (test('overwrites existing session file', () => { + const dir = createTempSessionDir(); + try { + const sessionPath = path.join(dir, 'overwrite-test.tmp'); + fs.writeFileSync(sessionPath, 'old content'); + const result = sessionManager.writeSessionContent(sessionPath, 'new content'); + assert.strictEqual(result, true); + assert.strictEqual(fs.readFileSync(sessionPath, 'utf8'), 'new content'); + } finally { + cleanup(dir); + } + })) passed++; else failed++; + + if (test('writeSessionContent returns false for invalid path', () => { + const result = sessionManager.writeSessionContent('/nonexistent/deep/path/session.tmp', 'content'); + assert.strictEqual(result, false, 'Should return false for invalid path'); + })) passed++; else failed++; + + // appendSessionContent tests + console.log('\nappendSessionContent:'); + + if (test('appends to existing session file', () => { + const dir = createTempSessionDir(); + try { + const sessionPath = path.join(dir, 'append-test.tmp'); + fs.writeFileSync(sessionPath, '# Session\n'); + const result = sessionManager.appendSessionContent(sessionPath, '\n## Added Section\n'); + assert.strictEqual(result, true); + const content = fs.readFileSync(sessionPath, 'utf8'); + assert.ok(content.includes('# Session')); + assert.ok(content.includes('## Added Section')); + } finally { + cleanup(dir); + } + })) passed++; else failed++; + + // deleteSession tests + console.log('\ndeleteSession:'); + + if (test('deletes existing session file', () => { + const dir = createTempSessionDir(); + try { + const sessionPath = path.join(dir, 'delete-me.tmp'); + fs.writeFileSync(sessionPath, '# To Delete'); + assert.ok(fs.existsSync(sessionPath), 'File should exist before delete'); + const result = sessionManager.deleteSession(sessionPath); + assert.strictEqual(result, true, 'Should return true'); + assert.ok(!fs.existsSync(sessionPath), 'File should not exist after delete'); + } finally { + cleanup(dir); + } + })) passed++; else failed++; + + if (test('deleteSession returns false for non-existent file', () => { + const result = sessionManager.deleteSession('/nonexistent/session.tmp'); + assert.strictEqual(result, false, 'Should return false for missing file'); + })) passed++; else failed++; + + // sessionExists tests + console.log('\nsessionExists:'); + + if (test('returns true for existing session file', () => { + const dir = createTempSessionDir(); + try { + const sessionPath = path.join(dir, 'exists.tmp'); + fs.writeFileSync(sessionPath, '# Exists'); + assert.strictEqual(sessionManager.sessionExists(sessionPath), true); + } finally { + cleanup(dir); + } + })) passed++; else failed++; + + if (test('returns false for non-existent file', () => { + assert.strictEqual(sessionManager.sessionExists('/nonexistent/file.tmp'), false); + })) passed++; else failed++; + + if (test('returns false for directory (not a file)', () => { + const dir = createTempSessionDir(); + try { + assert.strictEqual(sessionManager.sessionExists(dir), false, 'Directory should not count as session'); + } finally { + cleanup(dir); + } + })) passed++; else failed++; + + // getSessionStats with empty content + if (test('getSessionStats handles empty string content', () => { + const stats = sessionManager.getSessionStats(''); + assert.strictEqual(stats.totalItems, 0); + // Empty string is falsy in JS, so content ? ... : 0 returns 0 + assert.strictEqual(stats.lineCount, 0, 'Empty string is falsy, lineCount = 0'); + assert.strictEqual(stats.hasNotes, false); + assert.strictEqual(stats.hasContext, false); + })) passed++; else failed++; + // Cleanup — restore both HOME and USERPROFILE (Windows) process.env.HOME = origHome; if (origUserProfile !== undefined) {