mirror of
https://github.com/Jeuners/ECC.git
synced 2026-09-14 09:08:12 +02:00
fix(hooks,lib): fix hook detection and parsing edge cases (#2405)
* fix(hooks,lib): fix hook detection and parsing edge cases
- auto-tmux-dev: dev\b -> dev(?![\w-]) so one-shot dev-build/dev-docs scripts
are not detached into tmux; align command shapes (yarn run dev, bun dev) with
pre-bash-dev-server-block.js DEV_PATTERN.
- pre-bash-commit-quality: skip obvious non-secret placeholders (env refs,
${...}, <...>, whitelisted tokens) in the api-key rule without suppressing
real high-entropy secrets; make -m message extraction quote- and
escaped-quote-aware so `-m "fix: \"x\""` / apostrophes are not truncated.
- pre-compact: annotate the CURRENT worktree's session (match **Worktree:** /
legacy **Project:**) instead of the newest *-session.tmp across all projects,
layered onto the LLM-summary flow from #2388; a present-but-blank Worktree
header is treated as non-legacy (no foreign project fallback).
- shell-substitution: stop double-appending a trailing backslash in an
unterminated backtick span.
- utils readStdinJson: on overflow, settle and resolve {} immediately (clear
timer + listeners) instead of waiting for end/timeout and parsing a partial
prefix; surface the overflow on stderr.
Regression tests added/extended (new tests/hooks/pre-compact.test.js).
Addresses review feedback on #2405. The earlier block-no-verify change was
dropped: its message-value skip on merge/cherry-pick/am/rebase would let
`git rebase -m --no-verify` bypass the hook (rebase's -m is the boolean
--merge), a false-negative worse than the contrived false-positive it fixed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ci): align hook fixtures and drain oversized stdin
---------
Co-authored-by: djpjronline-netizen <276112803+djpjronline-netizen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com>
This commit is contained in:
djpjronline-netizen
GitHub
parent
6be87a56ae
commit
837acaf20b
11 changed files with 517 additions and 53 deletions
|
|
@ -119,6 +119,14 @@ function runTests() {
|
|||
assert.strictEqual(output.tool_input.command, 'npm run develop');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('does not transform npm run dev-build (hyphenated script)', () => {
|
||||
const input = { tool_input: { command: 'npm run dev-build' } };
|
||||
const result = runScript(input);
|
||||
assert.strictEqual(result.code, 0);
|
||||
const output = JSON.parse(result.stdout);
|
||||
assert.strictEqual(output.tool_input.command, 'npm run dev-build');
|
||||
})) passed++; else failed++;
|
||||
|
||||
console.log('\nEdge cases:');
|
||||
|
||||
if (test('handles empty input gracefully', () => {
|
||||
|
|
|
|||
|
|
@ -1247,7 +1247,9 @@ async function runTests() {
|
|||
|
||||
// Create an active .tmp session file
|
||||
const sessionFile = path.join(sessionsDir, '2026-02-11-test-session.tmp');
|
||||
fs.writeFileSync(sessionFile, '# Session: 2026-02-11\n**Started:** 10:00\n');
|
||||
fs.writeFileSync(sessionFile, buildSessionStartFixture('**Started:** 10:00', {
|
||||
title: '# Session: 2026-02-11'
|
||||
}));
|
||||
|
||||
try {
|
||||
await runScript(path.join(scriptsDir, 'pre-compact.js'), '', {
|
||||
|
|
@ -3761,7 +3763,7 @@ async function runTests() {
|
|||
// Create a session .tmp file and a non-session .tmp file
|
||||
const sessionFile = path.join(sessionsDir, '2026-02-11-abc-session.tmp');
|
||||
const otherTmpFile = path.join(sessionsDir, 'other-data.tmp');
|
||||
fs.writeFileSync(sessionFile, '# Session\n');
|
||||
fs.writeFileSync(sessionFile, buildSessionStartFixture('', { title: '# Session' }));
|
||||
fs.writeFileSync(otherTmpFile, 'some other data\n');
|
||||
|
||||
try {
|
||||
|
|
@ -4676,11 +4678,11 @@ async function runTests() {
|
|||
passed++;
|
||||
else failed++;
|
||||
|
||||
// Round 41: pre-compact.js (multiple session files)
|
||||
// Round 41: pre-compact.js (multiple sessions for the current worktree)
|
||||
console.log('\nRound 41: pre-compact.js (multiple session files):');
|
||||
|
||||
if (
|
||||
await asyncTest('annotates only the newest session file when multiple exist', async () => {
|
||||
await asyncTest('annotates only the newest session when multiple match the current worktree', async () => {
|
||||
const isoHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-compact-multi-'));
|
||||
const sessionsDir = getCanonicalSessionsDir(isoHome);
|
||||
fs.mkdirSync(sessionsDir, { recursive: true });
|
||||
|
|
@ -4688,11 +4690,12 @@ async function runTests() {
|
|||
// Create two session files with different mtimes
|
||||
const olderSession = path.join(sessionsDir, '2026-01-01-older-session.tmp');
|
||||
const newerSession = path.join(sessionsDir, '2026-02-11-newer-session.tmp');
|
||||
fs.writeFileSync(olderSession, '# Older Session\n');
|
||||
const olderContent = buildSessionStartFixture('', { title: '# Older Session' });
|
||||
fs.writeFileSync(olderSession, olderContent);
|
||||
// Small delay to ensure different mtime
|
||||
const now = Date.now();
|
||||
fs.utimesSync(olderSession, new Date(now - 60000), new Date(now - 60000));
|
||||
fs.writeFileSync(newerSession, '# Newer Session\n');
|
||||
fs.writeFileSync(newerSession, buildSessionStartFixture('', { title: '# Newer Session' }));
|
||||
|
||||
try {
|
||||
const result = await runScript(path.join(scriptsDir, 'pre-compact.js'), '', {
|
||||
|
|
@ -4702,11 +4705,11 @@ async function runTests() {
|
|||
assert.strictEqual(result.code, 0);
|
||||
|
||||
const newerContent = fs.readFileSync(newerSession, 'utf8');
|
||||
const olderContent = fs.readFileSync(olderSession, 'utf8');
|
||||
const updatedOlderContent = fs.readFileSync(olderSession, 'utf8');
|
||||
|
||||
// findFiles sorts by mtime newest first, so sessions[0] is the newest
|
||||
// findFiles sorts matches by mtime, so the newest matching worktree wins.
|
||||
assert.ok(newerContent.includes('Compaction occurred'), 'Should annotate the newest session file');
|
||||
assert.strictEqual(olderContent, '# Older Session\n', 'Should NOT annotate older session files');
|
||||
assert.strictEqual(updatedOlderContent, olderContent, 'Should NOT annotate older session files');
|
||||
} finally {
|
||||
fs.rmSync(isoHome, { recursive: true, force: true });
|
||||
}
|
||||
|
|
@ -6208,7 +6211,9 @@ Some random content without the expected ### Context to Load section
|
|||
|
||||
// Create a minimal session .tmp file
|
||||
const sessionFile = path.join(sessionsDir, '2026-01-01-test-session.tmp');
|
||||
fs.writeFileSync(sessionFile, '# Session: 2026-01-01\n');
|
||||
fs.writeFileSync(sessionFile, buildSessionStartFixture('', {
|
||||
title: '# Session: 2026-01-01'
|
||||
}));
|
||||
|
||||
// Create a minimal transcript with one user message
|
||||
const transcriptPath = path.join(testDir, 'transcript.jsonl');
|
||||
|
|
|
|||
|
|
@ -235,6 +235,40 @@ if (test('blocks commits with staged secret patterns across checkable files', ()
|
|||
});
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('blocks commits with an unquoted API key assignment', () => {
|
||||
inTempRepo(repoDir => {
|
||||
writeAndStage(repoDir, 'config.py', [
|
||||
'API_KEY=sk_live_1234567890abcdef',
|
||||
''
|
||||
].join('\n'));
|
||||
|
||||
const input = JSON.stringify({ tool_input: { command: 'git commit -m "fix: unquoted key"' } });
|
||||
const { result, stderr } = captureConsoleError(() => hook.evaluate(input));
|
||||
|
||||
assert.strictEqual(result.output, input);
|
||||
assert.strictEqual(result.exitCode, 2);
|
||||
assert.ok(stderr.includes('Potential API key'), `expected unquoted API key warning, got: ${stderr}`);
|
||||
});
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('does not flag ordinary unquoted apiKey code references', () => {
|
||||
inTempRepo(repoDir => {
|
||||
writeAndStage(repoDir, 'index.js', [
|
||||
'const apiKey = getApiKeyFromVault();',
|
||||
'this.apiKey = options.apiKey;',
|
||||
'const apiKey2 = process.env.API_KEY;',
|
||||
''
|
||||
].join('\n'));
|
||||
|
||||
const input = JSON.stringify({ tool_input: { command: 'git commit -m "fix: no secret here"' } });
|
||||
const { result, stderr } = captureConsoleError(() => hook.evaluate(input));
|
||||
|
||||
assert.strictEqual(result.output, input);
|
||||
assert.strictEqual(result.exitCode, 0, `expected exit 0 (no secrets), got ${result.exitCode}: ${stderr}`);
|
||||
assert.ok(!stderr.includes('Potential API key'), `should not flag ordinary code as a secret, got: ${stderr}`);
|
||||
});
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('reports eslint pylint and golint failures from staged files', () => {
|
||||
inTempRepo(repoDir => {
|
||||
writeAndStage(repoDir, 'index.js', 'const lint = true;\n');
|
||||
|
|
@ -291,5 +325,52 @@ if (test('stdin entry point truncates oversized input and preserves pass-through
|
|||
assert.ok(result.stderr.includes('[Hook] Error:'), 'truncated JSON should be logged and allowed');
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Secret-scanner placeholder exclusion (false-positive fix, no false-negative) ---
|
||||
|
||||
if (test('isPlaceholderSecret suppresses obvious non-secret placeholders', () => {
|
||||
for (const v of ['process.env.API_KEY', '${API_KEY}', '<YOUR_KEY>', 'REPLACE_ME', 'CHANGEME', 'YOUR_API_KEY', '']) {
|
||||
assert.strictEqual(hook.isPlaceholderSecret(v), true, `should suppress placeholder: ${JSON.stringify(v)}`);
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('isPlaceholderSecret does NOT suppress real high-entropy secrets', () => {
|
||||
for (const v of [
|
||||
'sk-live-abcdef0123456789ABCDEF', // prefixed
|
||||
'9F8A7B6C5D4E3F2A1B0C9D8E7F6A5B4C', // uppercase hex
|
||||
'JBSWY3DPEHPK3PXP', // base32 TOTP/HMAC seed
|
||||
'1234567890123456', // digit-only token
|
||||
'PROD_7F3A9C2E_LIVE_8821', // uppercase-with-underscore token
|
||||
'AbCd1234EfGh5678' // mixed token
|
||||
]) {
|
||||
assert.strictEqual(hook.isPlaceholderSecret(v), false, `must NOT suppress real secret: ${v}`);
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Quote-aware commit-message extraction (truncation fix) ---
|
||||
|
||||
if (test('captures full double-quoted -m message containing an apostrophe', () => {
|
||||
const res = hook.validateCommitMessage(`git commit -m "fix: don't crash on empty input"`);
|
||||
assert.ok(res, 'expected a validation result');
|
||||
assert.strictEqual(res.message, "fix: don't crash on empty input");
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('captures full single-quoted -m message containing a double quote', () => {
|
||||
const res = hook.validateCommitMessage(`git commit -m 'fix: handle the "edge" case'`);
|
||||
assert.strictEqual(res.message, 'fix: handle the "edge" case');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('captures full double-quoted -m message with escaped inner quotes (not truncated)', () => {
|
||||
const res = hook.validateCommitMessage('git commit -m "fix: say \\"hello\\" to the user"');
|
||||
assert.ok(res, 'expected a validation result');
|
||||
assert.strictEqual(res.message, 'fix: say \\"hello\\" to the user');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('measures length of the full message past an apostrophe (not the truncated prefix)', () => {
|
||||
const subject = "fix: it's a deliberately long commit subject that comfortably exceeds seventy-two chars";
|
||||
const res = hook.validateCommitMessage(`git commit -m "${subject}"`);
|
||||
assert.strictEqual(res.message, subject);
|
||||
assert.ok(res.issues.some(i => i.type === 'length'), 'full (>72) message should trigger a length issue');
|
||||
})) passed++; else failed++;
|
||||
|
||||
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
|
|
|
|||
105
tests/hooks/pre-compact.test.js
Normal file
105
tests/hooks/pre-compact.test.js
Normal file
|
|
@ -0,0 +1,105 @@
|
|||
'use strict';
|
||||
/**
|
||||
* Tests for scripts/hooks/pre-compact.js — worktree-aware active-session
|
||||
* selection. The sessions dir is shared across projects/worktrees, so the
|
||||
* hook must annotate the CURRENT worktree's session, not whichever file is
|
||||
* newest by mtime. selectActiveSessionPath takes an injectable reader so the
|
||||
* selection logic is tested without touching the filesystem.
|
||||
*/
|
||||
|
||||
const assert = require('assert');
|
||||
const { selectActiveSessionPath } = require('../../scripts/hooks/pre-compact');
|
||||
|
||||
let passed = 0;
|
||||
let failed = 0;
|
||||
|
||||
function test(name, fn) {
|
||||
try {
|
||||
fn();
|
||||
console.log(` ✓ ${name}`);
|
||||
return true;
|
||||
} catch (err) {
|
||||
console.log(` ✗ ${name}`);
|
||||
console.log(` ${err.message}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// Reader built from a path -> content map (returns null for unknown/unreadable).
|
||||
function reader(map) {
|
||||
return (p) => (Object.prototype.hasOwnProperty.call(map, p) ? map[p] : null);
|
||||
}
|
||||
|
||||
const A = '/ecc-pre-compact-test/work/projA';
|
||||
const B = '/ecc-pre-compact-test/work/projB';
|
||||
|
||||
if (test('selects the session matching the current worktree, not the newest', () => {
|
||||
const sessions = [
|
||||
{ path: '/sessions/newest-session.tmp' }, // newest, different worktree
|
||||
{ path: '/sessions/older-session.tmp' }, // older, our worktree
|
||||
];
|
||||
const map = {
|
||||
'/sessions/newest-session.tmp': `**Project:** projB\n**Worktree:** ${B}\n`,
|
||||
'/sessions/older-session.tmp': `**Project:** projA\n**Worktree:** ${A}\n`,
|
||||
};
|
||||
assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), '/sessions/older-session.tmp');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('returns null when no session matches the current worktree (no foreign write)', () => {
|
||||
const sessions = [{ path: '/sessions/b-session.tmp' }];
|
||||
const map = { '/sessions/b-session.tmp': `**Project:** projB\n**Worktree:** ${B}\n` };
|
||||
assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), null);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('falls back to a legacy session (no Worktree header) with matching project name', () => {
|
||||
const sessions = [{ path: '/sessions/legacy-session.tmp' }];
|
||||
const map = { '/sessions/legacy-session.tmp': '**Project:** projA\n' };
|
||||
assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), '/sessions/legacy-session.tmp');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('does not project-match a session that has an explicit non-matching Worktree', () => {
|
||||
const sessions = [{ path: '/sessions/x-session.tmp' }];
|
||||
const map = { '/sessions/x-session.tmp': `**Project:** projA\n**Worktree:** ${B}\n` };
|
||||
assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), null);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('does not project-match a session whose Worktree header is present but blank', () => {
|
||||
// A blank/whitespace Worktree header is NOT a legacy session, so it must not
|
||||
// fall back to project-name matching and attach to a foreign session.
|
||||
const sessions = [{ path: '/sessions/blank-session.tmp' }];
|
||||
const map = { '/sessions/blank-session.tmp': '**Project:** projA\n**Worktree:** \n' };
|
||||
assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), null);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('does not project-match a session whose Worktree header has no value and no space', () => {
|
||||
// Same as above but the header is bare `**Worktree:**\n` (no trailing space) —
|
||||
// (.+) would have missed this; (.*) registers it as a present-but-empty header.
|
||||
const sessions = [{ path: '/sessions/blank-header.tmp' }];
|
||||
const map = { '/sessions/blank-header.tmp': '**Project:** projA\n**Worktree:**\n' };
|
||||
assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), null);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('worktree match wins over a newer session AND over a legacy project match', () => {
|
||||
const sessions = [
|
||||
{ path: '/sessions/legacy-session.tmp' }, // newest, legacy, same project
|
||||
{ path: '/sessions/wt-session.tmp' }, // older, exact worktree
|
||||
];
|
||||
const map = {
|
||||
'/sessions/legacy-session.tmp': '**Project:** projA\n',
|
||||
'/sessions/wt-session.tmp': `**Worktree:** ${A}\n`,
|
||||
};
|
||||
assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), '/sessions/wt-session.tmp');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('skips unreadable session files', () => {
|
||||
const sessions = [{ path: '/sessions/bad-session.tmp' }, { path: '/sessions/good-session.tmp' }];
|
||||
const map = { '/sessions/good-session.tmp': `**Worktree:** ${A}\n` };
|
||||
assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), '/sessions/good-session.tmp');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('returns null for an empty session list', () => {
|
||||
assert.strictEqual(selectActiveSessionPath([], A, 'projA', reader({})), null);
|
||||
})) passed++; else failed++;
|
||||
|
||||
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
Loading…
Add table
Add a link
Reference in a new issue