fix(suggest-compact): clean up old counter temp files (affaan-m#2159)

* fix(suggest-compact): clean up old counter temp files

claude-tool-count-<sessionId> files were written into the OS temp dir
on every hook run and never removed, accumulating one orphan per
session indefinitely.

Sweep stale counter files at the top of main() before opening the
active counter. Retention is env-tunable via COMPACT_STATE_TTL_DAYS
(default 14 days); invalid values fall back to the default. The
active session's counter file is preserved unconditionally even if
its mtime is past the cutoff. Failures during the sweep are swallowed
to preserve the always-exit-0 hook contract.

Adds 7 regression tests covering the sweep, env-var validation, and
the always-exit-0 invariant under a populated temp dir.

Fixes affaan-m#2156

* fix(suggest-compact): preserve counter files at the TTL cutoff boundary

The cleanup sweep used `mtimeMs > cutoffMs` to short-circuit, which
matched files whose mtime sits exactly on the cutoff boundary and
deleted them. The cleanupOldCounters docstring promises only files
*older than* retentionDays are removed; a file at age == retentionDays
is not older than retentionDays, so it must survive.

Switch the comparison to `>=` so only strictly older files fall
through to deletion. Add a regression test that pins boundary-aged
files (mtimeMs sitting just past the projected cutoff) are preserved.

Refs affaan-m#2156

Author: gaurav0107

scripts/hooks/suggest-compact.js

@@ -23,6 +23,72 @@ const {
   output
 } = require('../lib/utils');
 
+const COUNTER_FILE_PREFIX = 'claude-tool-count-';
+const DEFAULT_COMPACT_STATE_TTL_DAYS = 14;
+
+function getCounterRetentionDays() {
+  const raw = process.env.COMPACT_STATE_TTL_DAYS;
+  if (!raw) return DEFAULT_COMPACT_STATE_TTL_DAYS;
+  const parsed = Number.parseInt(raw, 10);
+  return Number.isInteger(parsed) && parsed > 0 ? parsed : DEFAULT_COMPACT_STATE_TTL_DAYS;
+}
+
+/**
+ * Sweep stale counter files from the temp dir.
+ *
+ * Each session writes `claude-tool-count-<sessionId>` into the OS temp
+ * dir; nothing else removes them. Without a sweep these files accumulate
+ * one-per-session forever. This helper removes counters whose mtime is
+ * older than `retentionDays`, while preserving the active session's
+ * counter (which is about to be re-written by the caller).
+ *
+ * The helper never throws; per the always-exit-0 hook contract any
+ * filesystem failure is swallowed and logged to stderr.
+ *
+ * @param {string} tempDir - The temp directory to sweep.
+ * @param {number} retentionDays - Files older than this many days are removed.
+ * @param {string} currentCounterFile - Absolute path of the active session's
+ *   counter file; preserved unconditionally.
+ */
+function cleanupOldCounters(tempDir, retentionDays, currentCounterFile) {
+  let entries;
+  try {
+    entries = fs.readdirSync(tempDir, { withFileTypes: true });
+  } catch (err) {
+    log(`[StrategicCompact] Skipping counter sweep; readdir failed: ${err.message}`);
+    return;
+  }
+
+  const cutoffMs = Date.now() - retentionDays * 24 * 60 * 60 * 1000;
+  const currentBasename = path.basename(currentCounterFile);
+
+  for (const entry of entries) {
+    if (!entry.isFile()) continue;
+    if (!entry.name.startsWith(COUNTER_FILE_PREFIX)) continue;
+    if (entry.name === currentBasename) continue;
+
+    const fullPath = path.join(tempDir, entry.name);
+    let stats;
+    try {
+      stats = fs.statSync(fullPath);
+    } catch {
+      continue;
+    }
+
+    // Strict "older than" semantics per the docstring: a file whose mtime
+    // sits exactly on the cutoff boundary has age == retentionDays, which
+    // is not *older than* retentionDays, so preserve it. Use >= so only
+    // strictly older files (mtimeMs < cutoffMs) fall through to deletion.
+    if (stats.mtimeMs >= cutoffMs) continue;
+
+    try {
+      fs.rmSync(fullPath, { force: true });
+    } catch (err) {
+      log(`[StrategicCompact] Warning: failed to prune stale counter ${fullPath}: ${err.message}`);
+    }
+  }
+}
+
 async function resolveSessionId() {
   // Claude Code passes hook input via stdin JSON; session_id is the
   // canonical field. Fall back to the legacy env var, then 'default'.
@@ -43,7 +109,13 @@ async function main() {
   // legacy env var, or 'default' as fallback.
   const rawSessionId = await resolveSessionId();
   const sessionId = rawSessionId.replace(/[^a-zA-Z0-9_-]/g, '') || 'default';
-  const counterFile = path.join(getTempDir(), `claude-tool-count-${sessionId}`);
+  const tempDir = getTempDir();
+  const counterFile = path.join(tempDir, `${COUNTER_FILE_PREFIX}${sessionId}`);
+
+  // Sweep stale counter files (concern 1 of #2156). Cheap, swallows errors,
+  // skips the active session's file. See cleanupOldCounters for details.
+  cleanupOldCounters(tempDir, getCounterRetentionDays(), counterFile);
+
   const rawThreshold = parseInt(process.env.COMPACT_THRESHOLD || '50', 10);
   const threshold = Number.isFinite(rawThreshold) && rawThreshold > 0 && rawThreshold <= 10000
     ? rawThreshold

tests/hooks/suggest-compact.test.js

@@ -451,6 +451,192 @@ function runTests() {
   })) passed++;
   else failed++;
 
+  // ── Counter file cleanup (#2156) ──
+  // claude-tool-count-<sessionId> files were never removed. The hook now
+  // sweeps stale counters older than COMPACT_STATE_TTL_DAYS (default 14)
+  // before opening the active counter. These tests pin the contract.
+  console.log('\nCounter file cleanup (#2156):');
+
+  /**
+   * Set a file's mtime/atime to N days ago.
+   */
+  function setMtimeDaysAgo(filePath, daysAgo) {
+    const seconds = Math.floor(Date.now() / 1000) - daysAgo * 24 * 60 * 60;
+    fs.utimesSync(filePath, seconds, seconds);
+  }
+
+  if (test('removes counter files older than retention window', () => {
+    const { sessionId, cleanup } = createCounterContext();
+    const stale = getCounterFilePath(`stale-${Date.now()}`);
+    fs.writeFileSync(stale, '1');
+    setMtimeDaysAgo(stale, 30);
+    try {
+      const result = runCompact({ CLAUDE_SESSION_ID: sessionId });
+      assert.strictEqual(result.code, 0, 'Should exit 0');
+      assert.ok(!fs.existsSync(stale),
+        `Stale counter file should have been swept. Path: ${stale}`);
+    } finally {
+      try { fs.unlinkSync(stale); } catch (_err) { /* ignore */ }
+      cleanup();
+    }
+  })) passed++;
+  else failed++;
+
+  if (test('preserves counter files within retention window', () => {
+    const { sessionId, cleanup } = createCounterContext();
+    const fresh = getCounterFilePath(`fresh-${Date.now()}`);
+    fs.writeFileSync(fresh, '1');
+    setMtimeDaysAgo(fresh, 5);
+    try {
+      runCompact({ CLAUDE_SESSION_ID: sessionId });
+      assert.ok(fs.existsSync(fresh),
+        `Fresh counter file should be preserved. Path: ${fresh}`);
+    } finally {
+      try { fs.unlinkSync(fresh); } catch (_err) { /* ignore */ }
+      cleanup();
+    }
+  })) passed++;
+  else failed++;
+
+  if (test('preserves the active session\'s counter file even if old', () => {
+    const { sessionId, counterFile, cleanup } = createCounterContext();
+    cleanup();
+    fs.writeFileSync(counterFile, '7');
+    setMtimeDaysAgo(counterFile, 30);
+    try {
+      const result = runCompact({ CLAUDE_SESSION_ID: sessionId });
+      assert.strictEqual(result.code, 0, 'Should exit 0');
+      // Active counter survives the sweep AND is incremented by the hook (7 -> 8).
+      assert.ok(fs.existsSync(counterFile),
+        'Active session counter must survive the sweep');
+      const count = parseInt(fs.readFileSync(counterFile, 'utf8').trim(), 10);
+      assert.strictEqual(count, 8,
+        `Active counter should be incremented by the hook. Got ${count}`);
+    } finally {
+      cleanup();
+    }
+  })) passed++;
+  else failed++;
+
+  if (test('honours COMPACT_STATE_TTL_DAYS env var', () => {
+    const { sessionId, cleanup } = createCounterContext();
+    const target = getCounterFilePath(`ttl-${Date.now()}`);
+    fs.writeFileSync(target, '1');
+    setMtimeDaysAgo(target, 5); // Within default 14d window, but outside TTL=3
+    try {
+      const result = runCompact({
+        CLAUDE_SESSION_ID: sessionId,
+        COMPACT_STATE_TTL_DAYS: '3'
+      });
+      assert.strictEqual(result.code, 0);
+      assert.ok(!fs.existsSync(target),
+        `TTL=3 should sweep a 5-day-old file. Path: ${target}`);
+    } finally {
+      try { fs.unlinkSync(target); } catch (_err) { /* ignore */ }
+      cleanup();
+    }
+  })) passed++;
+  else failed++;
+
+  if (test('falls back to default for invalid COMPACT_STATE_TTL_DAYS', () => {
+    const { sessionId, cleanup } = createCounterContext();
+    const target = getCounterFilePath(`fallback-${Date.now()}`);
+    fs.writeFileSync(target, '1');
+    setMtimeDaysAgo(target, 5); // Within default 14d window, would survive a fallback
+    try {
+      // Each invalid form: zero, negative, non-numeric — should fall back to 14d default.
+      for (const bad of ['0', '-5', 'abc']) {
+        // Reset mtime each iteration so the file remains 5 days old.
+        setMtimeDaysAgo(target, 5);
+        const result = runCompact({
+          CLAUDE_SESSION_ID: sessionId,
+          COMPACT_STATE_TTL_DAYS: bad
+        });
+        assert.strictEqual(result.code, 0);
+        assert.ok(fs.existsSync(target),
+          `Invalid TTL '${bad}' should fall back to default (14d) and preserve a 5-day-old file`);
+      }
+    } finally {
+      try { fs.unlinkSync(target); } catch (_err) { /* ignore */ }
+      cleanup();
+    }
+  })) passed++;
+  else failed++;
+
+  if (test('does not touch unrelated temp files', () => {
+    const { sessionId, cleanup } = createCounterContext();
+    const unrelated = path.join(os.tmpdir(), `unrelated-${Date.now()}.tmp`);
+    fs.writeFileSync(unrelated, 'do not touch');
+    setMtimeDaysAgo(unrelated, 60);
+    try {
+      runCompact({ CLAUDE_SESSION_ID: sessionId });
+      assert.ok(fs.existsSync(unrelated),
+        `Unrelated temp file should not be swept. Path: ${unrelated}`);
+    } finally {
+      try { fs.unlinkSync(unrelated); } catch (_err) { /* ignore */ }
+      cleanup();
+    }
+  })) passed++;
+  else failed++;
+
+  if (test('preserves files whose mtime sits at or after the TTL cutoff', () => {
+    // Contract: docstring says files "older than" retentionDays are removed.
+    // A file at the exact boundary (age == retentionDays) is NOT older than
+    // retentionDays, so it must survive the sweep. Pins the >= comparison
+    // in cleanupOldCounters: anything with mtimeMs >= cutoffMs is skipped.
+    //
+    // We can't pin the boundary by clock — the sweep computes its own
+    // Date.now() after this test runs, so `setMtimeDaysAgo(file, 14)` is
+    // effectively "14d + handful of ms", placing the file just past the
+    // cutoff. To exercise the boundary deterministically, set the file's
+    // mtime two seconds *newer* than the projected cutoff: with `>` the
+    // file would be deleted (mtimeMs > cutoffMs is false at the cutoff
+    // edge); with `>=` it survives.
+    const { sessionId, cleanup } = createCounterContext();
+    const boundary = getCounterFilePath(`boundary-${Date.now()}`);
+    fs.writeFileSync(boundary, '1');
+    const retentionDays = 14;
+    const boundaryMs = Date.now() - retentionDays * 24 * 60 * 60 * 1000 + 2000;
+    const sec = Math.floor(boundaryMs / 1000);
+    fs.utimesSync(boundary, sec, sec);
+    try {
+      const result = runCompact({ CLAUDE_SESSION_ID: sessionId });
+      assert.strictEqual(result.code, 0);
+      assert.ok(fs.existsSync(boundary),
+        `Boundary-aged counter file should be preserved. Path: ${boundary}`);
+    } finally {
+      try { fs.unlinkSync(boundary); } catch (_err) { /* ignore */ }
+      cleanup();
+    }
+  })) passed++;
+  else failed++;
+
+  if (test('exit 0 holds when sweep encounters a populated temp dir', () => {
+    // Functional smoke: with a mix of stale, fresh, and unrelated files
+    // present, the hook must still exit 0 — the always-exit-0 contract
+    // takes precedence over sweep failures.
+    const { sessionId, cleanup } = createCounterContext();
+    const stale = getCounterFilePath(`mix-stale-${Date.now()}`);
+    const fresh = getCounterFilePath(`mix-fresh-${Date.now()}`);
+    const unrelated = path.join(os.tmpdir(), `mix-unrelated-${Date.now()}.tmp`);
+    fs.writeFileSync(stale, '1');
+    fs.writeFileSync(fresh, '1');
+    fs.writeFileSync(unrelated, '1');
+    setMtimeDaysAgo(stale, 30);
+    setMtimeDaysAgo(fresh, 1);
+    setMtimeDaysAgo(unrelated, 30);
+    try {
+      const result = runCompact({ CLAUDE_SESSION_ID: sessionId });
+      assert.strictEqual(result.code, 0, 'Hook must exit 0 even with files in temp dir');
+    } finally {
+      for (const p of [stale, fresh, unrelated]) {
+        try { fs.unlinkSync(p); } catch (_err) { /* ignore */ }
+      }
+      cleanup();
+    }
+  })) passed++;
+  else failed++;
+
   // Summary
   console.log(`
 Results: Passed: ${passed}, Failed: ${failed}`);