From 9c5ca92e6e85eef6d784495fc9a0800d087d0901 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Wed, 25 Mar 2026 03:44:03 -0400 Subject: [PATCH] fix: finish hook fallback and canonical session follow-ups --- commands/save-session.md | 10 ++++---- scripts/hooks/run-with-flags.js | 27 +++++++++++++++++--- scripts/hooks/session-start.js | 34 ++++++++++++++++++++++--- tests/hooks/hooks.test.js | 10 +++++--- tests/integration/hooks.test.js | 1 + tests/lib/session-manager.test.js | 15 +++++++---- tests/lib/utils.test.js | 9 +++++++ tests/scripts/codex-hooks.test.js | 16 +++++++++--- tests/scripts/sync-ecc-to-codex.test.js | 14 +++++++--- 9 files changed, 108 insertions(+), 28 deletions(-) diff --git a/commands/save-session.md b/commands/save-session.md index d67a4e60..3ac78936 100644 --- a/commands/save-session.md +++ b/commands/save-session.md @@ -36,12 +36,12 @@ mkdir -p ~/.claude/session-data Create `~/.claude/session-data/YYYY-MM-DD--session.tmp`, using today's actual date and a short-id that satisfies the rules enforced by `SESSION_FILENAME_REGEX` in `session-manager.js`: -- Allowed characters: lowercase `a-z`, digits `0-9`, hyphens `-` -- Minimum length: 8 characters -- No uppercase letters, no underscores, no spaces +- Compatibility characters: letters `a-z` / `A-Z`, digits `0-9`, hyphens `-`, underscores `_` +- Compatibility minimum length: 1 character +- Recommended style for new files: lowercase letters, digits, and hyphens with 8+ characters to avoid collisions -Valid examples: `abc123de`, `a1b2c3d4`, `frontend-worktree-1` -Invalid examples: `ABC123de` (uppercase), `short` (under 8 chars), `test_id1` (underscore) +Valid examples: `abc123de`, `a1b2c3d4`, `frontend-worktree-1`, `ChezMoi_2` +Avoid for new files: `A`, `test_id1`, `ABC123de` Full valid filename example: `2024-01-15-abc123de-session.tmp` diff --git a/scripts/hooks/run-with-flags.js b/scripts/hooks/run-with-flags.js index e2376eed..76c88315 100755 --- a/scripts/hooks/run-with-flags.js +++ b/scripts/hooks/run-with-flags.js @@ -66,6 +66,16 @@ function emitHookResult(raw, output) { return 0; } +function writeLegacySpawnOutput(raw, result) { + const stdout = typeof result.stdout === 'string' ? result.stdout : ''; + if (stdout) { + process.stdout.write(stdout); + return; + } + + process.stdout.write(raw); +} + function getPluginRoot() { if (process.env.CLAUDE_PLUGIN_ROOT && process.env.CLAUDE_PLUGIN_ROOT.trim()) { return process.env.CLAUDE_PLUGIN_ROOT; @@ -135,7 +145,7 @@ async function main() { } // Legacy path: spawn a child Node process for hooks without run() export - const result = spawnSync('node', [scriptPath], { + const result = spawnSync(process.execPath, [scriptPath], { input: raw, encoding: 'utf8', env: { @@ -147,11 +157,20 @@ async function main() { timeout: 30000 }); - if (result.stdout) process.stdout.write(result.stdout); + writeLegacySpawnOutput(raw, result); if (result.stderr) process.stderr.write(result.stderr); - const code = Number.isInteger(result.status) ? result.status : 0; - process.exit(code); + if (result.error || result.signal || result.status === null) { + const failureDetail = result.error + ? result.error.message + : result.signal + ? `terminated by signal ${result.signal}` + : 'missing exit status'; + writeStderr(`[Hook] legacy hook execution failed for ${hookId}: ${failureDetail}`); + process.exit(1); + } + + process.exit(Number.isInteger(result.status) ? result.status : 0); } main().catch(err => { diff --git a/scripts/hooks/session-start.js b/scripts/hooks/session-start.js index ac57dc9d..a9943080 100644 --- a/scripts/hooks/session-start.js +++ b/scripts/hooks/session-start.js @@ -22,6 +22,36 @@ const { const { getPackageManager, getSelectionPrompt } = require('../lib/package-manager'); const { listAliases } = require('../lib/session-aliases'); const { detectProjectType } = require('../lib/project-detect'); +const path = require('path'); + +function dedupeRecentSessions(searchDirs) { + const recentSessionsByName = new Map(); + + for (const [dirIndex, dir] of searchDirs.entries()) { + const matches = findFiles(dir, '*-session.tmp', { maxAge: 7 }); + + for (const match of matches) { + const basename = path.basename(match.path); + const current = { + ...match, + basename, + dirIndex, + }; + const existing = recentSessionsByName.get(basename); + + if ( + !existing + || current.mtime > existing.mtime + || (current.mtime === existing.mtime && current.dirIndex < existing.dirIndex) + ) { + recentSessionsByName.set(basename, current); + } + } + } + + return Array.from(recentSessionsByName.values()) + .sort((left, right) => right.mtime - left.mtime || left.dirIndex - right.dirIndex); +} async function main() { const sessionsDir = getSessionsDir(); @@ -33,9 +63,7 @@ async function main() { ensureDir(learnedDir); // Check for recent session files (last 7 days) - const recentSessions = getSessionSearchDirs() - .flatMap(dir => findFiles(dir, '*-session.tmp', { maxAge: 7 })) - .sort((a, b) => b.mtime - a.mtime); + const recentSessions = dedupeRecentSessions(getSessionSearchDirs()); if (recentSessions.length > 0) { const latest = recentSessions[0]; diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 7f171b74..f0c0d7d0 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -437,10 +437,12 @@ async function runTests() { const isoHome = path.join(os.tmpdir(), `ecc-canonical-start-${Date.now()}`); const canonicalDir = getCanonicalSessionsDir(isoHome); const legacyDir = getLegacySessionsDir(isoHome); - const filename = '2026-02-11-dupe1234-session.tmp'; + const now = new Date(); + const filename = `${now.toISOString().slice(0, 10)}-dupe1234-session.tmp`; const canonicalFile = path.join(canonicalDir, filename); const legacyFile = path.join(legacyDir, filename); - const sameTime = new Date('2026-02-11T12:00:00Z'); + const canonicalTime = new Date(now.getTime() - 60 * 1000); + const legacyTime = new Date(now.getTime() - 120 * 1000); fs.mkdirSync(canonicalDir, { recursive: true }); fs.mkdirSync(legacyDir, { recursive: true }); @@ -448,8 +450,8 @@ async function runTests() { fs.writeFileSync(canonicalFile, '# Canonical Session\n\nUse the canonical session-data copy.\n'); fs.writeFileSync(legacyFile, '# Legacy Session\n\nDo not prefer the legacy duplicate.\n'); - fs.utimesSync(canonicalFile, sameTime, sameTime); - fs.utimesSync(legacyFile, sameTime, sameTime); + fs.utimesSync(canonicalFile, canonicalTime, canonicalTime); + fs.utimesSync(legacyFile, legacyTime, legacyTime); try { const result = await runScript(path.join(scriptsDir, 'session-start.js'), '', { diff --git a/tests/integration/hooks.test.js b/tests/integration/hooks.test.js index d7753325..7ac768ba 100644 --- a/tests/integration/hooks.test.js +++ b/tests/integration/hooks.test.js @@ -94,6 +94,7 @@ function getSessionStartPayload(stdout) { assert.ok(stdout.trim(), 'Expected SessionStart hook to emit stdout payload'); const payload = JSON.parse(stdout); assert.strictEqual(payload.hookSpecificOutput?.hookEventName, 'SessionStart'); + assert.strictEqual(typeof payload.hookSpecificOutput?.additionalContext, 'string'); return payload; } diff --git a/tests/lib/session-manager.test.js b/tests/lib/session-manager.test.js index 116ff602..d2237b6f 100644 --- a/tests/lib/session-manager.test.js +++ b/tests/lib/session-manager.test.js @@ -341,8 +341,10 @@ src/main.ts // Override HOME to a temp dir for isolated getAllSessions/getSessionById tests // On Windows, os.homedir() uses USERPROFILE, not HOME — set both for cross-platform const tmpHome = path.join(os.tmpdir(), `ecc-session-mgr-test-${Date.now()}`); - const tmpSessionsDir = path.join(tmpHome, '.claude', 'sessions'); - fs.mkdirSync(tmpSessionsDir, { recursive: true }); + const tmpCanonicalSessionsDir = path.join(tmpHome, '.claude', 'session-data'); + const tmpLegacySessionsDir = path.join(tmpHome, '.claude', 'sessions'); + fs.mkdirSync(tmpCanonicalSessionsDir, { recursive: true }); + fs.mkdirSync(tmpLegacySessionsDir, { recursive: true }); const origHome = process.env.HOME; const origUserProfile = process.env.USERPROFILE; @@ -355,7 +357,10 @@ src/main.ts { name: '2026-02-10-session.tmp', content: '# Old format session' }, ]; for (let i = 0; i < testSessions.length; i++) { - const filePath = path.join(tmpSessionsDir, testSessions[i].name); + const targetDir = testSessions[i].name === '2026-02-10-session.tmp' + ? tmpLegacySessionsDir + : tmpCanonicalSessionsDir; + const filePath = path.join(targetDir, testSessions[i].name); fs.writeFileSync(filePath, testSessions[i].content); // Stagger modification times so sort order is deterministic const mtime = new Date(Date.now() - (testSessions.length - i) * 60000); @@ -423,8 +428,8 @@ src/main.ts })) passed++; else failed++; if (test('getAllSessions ignores non-.tmp files', () => { - fs.writeFileSync(path.join(tmpSessionsDir, 'notes.txt'), 'not a session'); - fs.writeFileSync(path.join(tmpSessionsDir, 'compaction-log.txt'), 'log'); + fs.writeFileSync(path.join(tmpCanonicalSessionsDir, 'notes.txt'), 'not a session'); + fs.writeFileSync(path.join(tmpCanonicalSessionsDir, 'compaction-log.txt'), 'log'); const result = sessionManager.getAllSessions({ limit: 100 }); assert.strictEqual(result.total, 5, 'Should only count .tmp session files'); })) passed++; else failed++; diff --git a/tests/lib/utils.test.js b/tests/lib/utils.test.js index 8040acec..5bcea414 100644 --- a/tests/lib/utils.test.js +++ b/tests/lib/utils.test.js @@ -146,6 +146,15 @@ function runTests() { assert.strictEqual(utils.sanitizeSessionId('my-project_123'), 'my-project_123'); })) passed++; else failed++; + if (test('sanitizeSessionId avoids Windows reserved device names', () => { + for (const reservedName of ['CON', 'prn', 'Aux', 'nul', 'COM1', 'lpt9']) { + const sanitized = utils.sanitizeSessionId(reservedName); + assert.ok(sanitized, `Expected sanitized output for ${reservedName}`); + assert.notStrictEqual(sanitized.toUpperCase(), reservedName.toUpperCase()); + assert.ok(/-[a-f0-9]{6}$/i.test(sanitized), `Expected deterministic hash suffix for ${reservedName}, got ${sanitized}`); + } + })) passed++; else failed++; + if (test('sanitizeSessionId returns null for empty or punctuation-only values', () => { assert.strictEqual(utils.sanitizeSessionId(''), null); assert.strictEqual(utils.sanitizeSessionId(null), null); diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 482e7428..0856edf5 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -32,8 +32,18 @@ function cleanup(dirPath) { fs.rmSync(dirPath, { recursive: true, force: true }); } +function toBashPath(filePath) { + if (process.platform !== 'win32') { + return filePath; + } + + return String(filePath) + .replace(/^([A-Za-z]):/, (_, driveLetter) => `/${driveLetter.toLowerCase()}`) + .replace(/\\/g, '/'); +} + function runBash(scriptPath, args = [], env = {}, cwd = repoRoot) { - return spawnSync('bash', [scriptPath, ...args], { + return spawnSync('bash', [toBashPath(scriptPath), ...args], { cwd, env: { ...process.env, @@ -64,8 +74,8 @@ if ( try { const result = runBash(installScript, [], { - HOME: homeDir, - ECC_GLOBAL_HOOKS_DIR: weirdHooksDir + HOME: toBashPath(homeDir), + ECC_GLOBAL_HOOKS_DIR: toBashPath(weirdHooksDir) }); assert.strictEqual(result.status, 0, result.stderr || result.stdout); diff --git a/tests/scripts/sync-ecc-to-codex.test.js b/tests/scripts/sync-ecc-to-codex.test.js index 58a4bb25..f6f2c51a 100644 --- a/tests/scripts/sync-ecc-to-codex.test.js +++ b/tests/scripts/sync-ecc-to-codex.test.js @@ -8,6 +8,11 @@ const path = require('path'); const scriptPath = path.join(__dirname, '..', '..', 'scripts', 'sync-ecc-to-codex.sh'); const source = fs.readFileSync(scriptPath, 'utf8'); +const runOrEchoStart = source.indexOf('run_or_echo() {'); +const runOrEchoEnd = source.indexOf('\n\nrequire_path() {', runOrEchoStart); +const runOrEchoSource = runOrEchoStart >= 0 && runOrEchoEnd > runOrEchoStart + ? source.slice(runOrEchoStart, runOrEchoEnd) + : ''; function test(name, fn) { try { @@ -28,15 +33,16 @@ function runTests() { let failed = 0; if (test('run_or_echo does not use eval', () => { - assert.ok(!source.includes('eval "$@"'), 'run_or_echo should not execute through eval'); + assert.ok(runOrEchoSource, 'Expected to locate run_or_echo function body'); + assert.ok(!runOrEchoSource.includes('eval "$@"'), 'run_or_echo should not execute through eval'); })) passed++; else failed++; if (test('run_or_echo executes argv directly', () => { - assert.ok(source.includes(' "$@"'), 'run_or_echo should execute the argv vector directly'); + assert.ok(runOrEchoSource.includes(' "$@"'), 'run_or_echo should execute the argv vector directly'); })) passed++; else failed++; if (test('dry-run output shell-escapes argv', () => { - assert.ok(source.includes(`printf ' %q' "$@"`), 'Dry-run mode should print shell-escaped argv'); + assert.ok(runOrEchoSource.includes(`printf ' %q' "$@"`), 'Dry-run mode should print shell-escaped argv'); })) passed++; else failed++; if (test('filesystem-changing calls use argv-form run_or_echo invocations', () => { @@ -49,4 +55,4 @@ function runTests() { process.exit(failed > 0 ? 1 : 0); } -runTests(); \ No newline at end of file +runTests();