From 8dc43e5f6075dc137981734c5de72c9e1de8f0a9 Mon Sep 17 00:00:00 2001 From: Gaurav Dubey Date: Sun, 7 Jun 2026 10:31:33 +0530 Subject: [PATCH] fix(session-start): support ECC_SESSION_RETENTION_DAYS opt-out + document env var (#2151) (#2163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(session-start): support ECC_SESSION_RETENTION_DAYS opt-out + document env var The retention pass for *-session.tmp files (issue #2151) landed previously, but the env var that controls it was undocumented in the README and rejected falsy values (0, off, disabled), silently falling back to the 30-day default. Users who want to keep all sessions for forensic or research workflows had no way to opt out. This patch: - Extends getSessionRetentionDays() so 0|off|false|disabled|never|none disables pruning entirely (returns null sentinel; default behavior unchanged). - Updates the call site in main() to skip pruneExpiredSessions when retention is null and emits a clear "[SessionStart] Pruning disabled via ECC_SESSION_RETENTION_DAYS" log line so the operator can tell pruning is off. - Documents ECC_SESSION_RETENTION_DAYS in the README "Hook Runtime Controls" section alongside the other ECC_SESSION_* knobs. - Adds three regression tests in tests/hooks/hooks.test.js covering opt-out via 0, opt-out via off, and garbage-value fallback to default 30. Verification: - node tests/hooks/hooks.test.js — 240/240 green (incl. 3 new retention tests) - node tests/run-all.js — 2622/2622 green - npx eslint scripts/hooks/session-start.js tests/hooks/hooks.test.js — clean - node scripts/ci/validate-no-personal-paths.js — clean - node scripts/ci/check-unicode-safety.js — clean - node scripts/ci/validate-hooks.js — 28 matchers validated - node scripts/ci/validate-rules.js — 115 files validated Fixes #2151 * docs(readme): list all ECC_SESSION_RETENTION_DAYS opt-out values + add Windows example Address reviewer feedback on PR #2163: - CodeRabbit and cubic both flagged that the README docs only listed 3 of 6 opt-out values accepted by getSessionRetentionDays() (0, off, disabled), while the implementation also accepts false, never, none. - cubic also flagged the missing Windows PowerShell example for the new variable, breaking the parallel structure of the existing ECC_CONTEXT_MONITOR_COST_WARNINGS example block. Updated the README to: - Spell out all six opt-out values (0, off, false, disabled, never, none) and clarify they "keep all sessions (disable pruning)". - Add an ECC_SESSION_RETENTION_DAYS line to the Windows PowerShell example. No behavior change. README only. Verification: - npx markdownlint README.md — clean - npx eslint scripts/hooks/session-start.js tests/hooks/hooks.test.js — clean --- README.md | 5 ++ scripts/hooks/session-start.js | 23 ++++++-- tests/hooks/hooks.test.js | 97 ++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c88aaf42..7f6c5102 100644 --- a/README.md +++ b/README.md @@ -479,6 +479,10 @@ export ECC_SESSION_START_MAX_CHARS=4000 # Disable SessionStart additional context entirely for low-context/local-model setups export ECC_SESSION_START_CONTEXT=off +# Session-tmp retention window in days (default: 30). +# Set to 0, off, false, disabled, never, or none to keep all sessions (disable pruning). +export ECC_SESSION_RETENTION_DAYS=14 + # Keep context/scope/loop warnings but suppress API-rate cost estimates export ECC_CONTEXT_MONITOR_COST_WARNINGS=off ``` @@ -487,6 +491,7 @@ Windows PowerShell: ```powershell [Environment]::SetEnvironmentVariable('ECC_CONTEXT_MONITOR_COST_WARNINGS', 'off', 'User') +[Environment]::SetEnvironmentVariable('ECC_SESSION_RETENTION_DAYS', '14', 'User') ``` --- diff --git a/scripts/hooks/session-start.js b/scripts/hooks/session-start.js index 4cdd1cf2..17bbc1a1 100644 --- a/scripts/hooks/session-start.js +++ b/scripts/hooks/session-start.js @@ -83,9 +83,22 @@ function dedupeRecentSessions(searchDirs) { .sort((left, right) => right.mtime - left.mtime || left.dirIndex - right.dirIndex); } +/** + * Resolve session retention days from the ECC_SESSION_RETENTION_DAYS env var. + * + * @returns {number|null} The retention window in days, or `null` when the + * user has explicitly opted out of pruning. Falsy/garbage values fall back + * to {@link DEFAULT_SESSION_RETENTION_DAYS}. + */ function getSessionRetentionDays() { const raw = process.env.ECC_SESSION_RETENTION_DAYS; if (!raw) return DEFAULT_SESSION_RETENTION_DAYS; + + const normalized = String(raw).trim().toLowerCase(); + if (['0', 'off', 'false', 'disabled', 'never', 'none'].includes(normalized)) { + return null; + } + const parsed = Number.parseInt(raw, 10); return Number.isInteger(parsed) && parsed > 0 ? parsed : DEFAULT_SESSION_RETENTION_DAYS; } @@ -526,9 +539,13 @@ async function main() { ensureDir(learnedDir); const retentionDays = getSessionRetentionDays(); - const prunedSessions = pruneExpiredSessions(sessionSearchDirs, retentionDays); - if (prunedSessions > 0) { - log(`[SessionStart] Pruned ${prunedSessions} expired session(s) older than ${retentionDays} day(s)`); + if (retentionDays === null) { + log('[SessionStart] Pruning disabled via ECC_SESSION_RETENTION_DAYS'); + } else { + const prunedSessions = pruneExpiredSessions(sessionSearchDirs, retentionDays); + if (prunedSessions > 0) { + log(`[SessionStart] Pruned ${prunedSessions} expired session(s) older than ${retentionDays} day(s)`); + } } const observerSessionId = resolveSessionId(); diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 7c69f988..7ed88422 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -5006,6 +5006,103 @@ async function runTests() { passed++; else failed++; + if ( + await asyncTest('disables pruning when ECC_SESSION_RETENTION_DAYS=0', async () => { + const isoHome = path.join(os.tmpdir(), `ecc-start-prune-off-${Date.now()}`); + const sessionsDir = getCanonicalSessionsDir(isoHome); + fs.mkdirSync(sessionsDir, { recursive: true }); + fs.mkdirSync(path.join(isoHome, '.claude', 'skills', 'learned'), { recursive: true }); + + const expiredFile = path.join(sessionsDir, '2026-01-01-keepme-session.tmp'); + fs.writeFileSync(expiredFile, '# Old Session\n\nSHOULD STILL EXIST'); + const ninetyDaysAgo = new Date(Date.now() - 90 * 24 * 60 * 60 * 1000); + fs.utimesSync(expiredFile, ninetyDaysAgo, ninetyDaysAgo); + + try { + const result = await runScript(path.join(scriptsDir, 'session-start.js'), '', { + HOME: isoHome, + USERPROFILE: isoHome, + ECC_SESSION_RETENTION_DAYS: '0', + }); + + assert.strictEqual(result.code, 0); + assert.ok(fs.existsSync(expiredFile), 'Should keep all sessions when retention is opt-out=0'); + assert.ok(result.stderr.includes('Pruning disabled via ECC_SESSION_RETENTION_DAYS'), + `Should log pruning disabled, stderr: ${result.stderr}`); + assert.ok(!result.stderr.includes('Pruned'), `Should not log any pruning, stderr: ${result.stderr}`); + } finally { + fs.rmSync(isoHome, { recursive: true, force: true }); + } + }) + ) + passed++; + else failed++; + + if ( + await asyncTest('disables pruning when ECC_SESSION_RETENTION_DAYS=off', async () => { + const isoHome = path.join(os.tmpdir(), `ecc-start-prune-offstr-${Date.now()}`); + const sessionsDir = getCanonicalSessionsDir(isoHome); + fs.mkdirSync(sessionsDir, { recursive: true }); + fs.mkdirSync(path.join(isoHome, '.claude', 'skills', 'learned'), { recursive: true }); + + const expiredFile = path.join(sessionsDir, '2025-12-15-keepme-session.tmp'); + fs.writeFileSync(expiredFile, '# Forensic Session\n\nKEEP ME'); + const sixtyDaysAgo = new Date(Date.now() - 60 * 24 * 60 * 60 * 1000); + fs.utimesSync(expiredFile, sixtyDaysAgo, sixtyDaysAgo); + + try { + const result = await runScript(path.join(scriptsDir, 'session-start.js'), '', { + HOME: isoHome, + USERPROFILE: isoHome, + ECC_SESSION_RETENTION_DAYS: 'off', + }); + + assert.strictEqual(result.code, 0); + assert.ok(fs.existsSync(expiredFile), 'Should keep all sessions when retention is opt-out=off'); + assert.ok(result.stderr.includes('Pruning disabled via ECC_SESSION_RETENTION_DAYS'), + `Should log pruning disabled, stderr: ${result.stderr}`); + } finally { + fs.rmSync(isoHome, { recursive: true, force: true }); + } + }) + ) + passed++; + else failed++; + + if ( + await asyncTest('falls back to default retention when ECC_SESSION_RETENTION_DAYS is garbage', async () => { + const isoHome = path.join(os.tmpdir(), `ecc-start-prune-garbage-${Date.now()}`); + const sessionsDir = getCanonicalSessionsDir(isoHome); + fs.mkdirSync(sessionsDir, { recursive: true }); + fs.mkdirSync(path.join(isoHome, '.claude', 'skills', 'learned'), { recursive: true }); + + const expiredFile = path.join(sessionsDir, '2026-01-01-pruneme-session.tmp'); + fs.writeFileSync(expiredFile, '# Old Session\n\nDELETE ME'); + const fortyDaysAgo = new Date(Date.now() - 40 * 24 * 60 * 60 * 1000); + fs.utimesSync(expiredFile, fortyDaysAgo, fortyDaysAgo); + + try { + const result = await runScript(path.join(scriptsDir, 'session-start.js'), '', { + HOME: isoHome, + USERPROFILE: isoHome, + ECC_SESSION_RETENTION_DAYS: 'bogus-value', + }); + + assert.strictEqual(result.code, 0); + assert.ok(!fs.existsSync(expiredFile), + 'Should fall back to default 30-day retention and prune the 40-day-old file'); + assert.ok(result.stderr.includes('Pruned 1 expired session'), + `Should log pruning at default retention, stderr: ${result.stderr}`); + assert.ok(!result.stderr.includes('Pruning disabled'), + 'Should NOT treat garbage as opt-out'); + } finally { + fs.rmSync(isoHome, { recursive: true, force: true }); + } + }) + ) + passed++; + else failed++; + console.log('\nRound 55: session-start.js (newest session selection):'); if (