From e7e38cd5086e7b1662b8c605dc696aa6cc21e8de Mon Sep 17 00:00:00 2001 From: bymle Date: Sun, 7 Jun 2026 13:25:36 +0800 Subject: [PATCH] fix(session-end): preserve $-sequences in user messages when rewriting summary (#2180) The regenerated summary block embeds raw user-message text and was passed as the *replacement* argument to String.prototype.replace, where $-sequences ($&, $$, $`, $') are special. A user message containing $& re-injected the entire matched block (duplicating the summary markers) and $$ collapsed to $, silently corrupting the persisted session summary. buildSummarySection only escapes newlines and backticks, not $. Fix: use function replacers (() => summaryBlock) at both rewrite sites so the replacement text is treated literally. Adds an end-to-end regression test. Co-authored-by: bymle <229636660+bymle@users.noreply.github.com> --- scripts/hooks/session-end.js | 8 ++- tests/hooks/session-end.test.js | 102 ++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 tests/hooks/session-end.test.js diff --git a/scripts/hooks/session-end.js b/scripts/hooks/session-end.js index d7ed8f59..8d0d4a28 100644 --- a/scripts/hooks/session-end.js +++ b/scripts/hooks/session-end.js @@ -255,16 +255,20 @@ async function main() { if (summary && updatedContent) { const summaryBlock = buildSummaryBlock(summary); + // Use function replacers: summaryBlock embeds raw user-message text, and a + // string replacement argument interprets $-sequences ($&, $$, $`, $', $n). + // A $& in a user message would otherwise re-inject the entire matched block + // and corrupt the persisted summary. A function replacer is treated literally. if (updatedContent.includes(SUMMARY_START_MARKER) && updatedContent.includes(SUMMARY_END_MARKER)) { updatedContent = updatedContent.replace( new RegExp(`${escapeRegExp(SUMMARY_START_MARKER)}[\\s\\S]*?${escapeRegExp(SUMMARY_END_MARKER)}`), - summaryBlock + () => summaryBlock ); } else { // Migration path for files created before summary markers existed. updatedContent = updatedContent.replace( /## (?:Session Summary|Current State)[\s\S]*?$/, - `${summaryBlock}\n\n### Notes for Next Session\n-\n\n### Context to Load\n\`\`\`\n[relevant files]\n\`\`\`\n` + () => `${summaryBlock}\n\n### Notes for Next Session\n-\n\n### Context to Load\n\`\`\`\n[relevant files]\n\`\`\`\n` ); } } diff --git a/tests/hooks/session-end.test.js b/tests/hooks/session-end.test.js new file mode 100644 index 00000000..9008674d --- /dev/null +++ b/tests/hooks/session-end.test.js @@ -0,0 +1,102 @@ +/** + * Tests for session-end.js hook + * + * Run with: node tests/hooks/session-end.test.js + */ + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const { getDateString, sanitizeSessionId } = require('../../scripts/lib/utils'); + +const script = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'session-end.js'); +const START = ''; +const END = ''; + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + return true; + } catch (err) { + console.log(` ✗ ${name}`); + console.log(` Error: ${err.message}`); + return false; + } +} + +function countOccurrences(haystack, needle) { + let n = 0; + let i = 0; + while ((i = haystack.indexOf(needle, i)) !== -1) { + n += 1; + i += needle.length; + } + return n; +} + +function runTests() { + console.log('\n=== Testing session-end.js ===\n'); + + let passed = 0; + let failed = 0; + + // Regression: a user message containing $-sequences ($&, $$, $`, $') must be + // written verbatim into the rewritten summary block. The block is fed to + // String.prototype.replace as the replacement argument, where those sequences + // are special — without escaping/a function replacer they corrupt the summary + // (e.g. $& injects the entire matched old block, duplicating the markers). + (test('preserves $-sequences in user messages when rewriting the summary block', () => { + // Isolate HOME so getSessionsDir() resolves under a temp dir. + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-session-end-')); + try { + const sessionsDir = path.join(home, '.claude', 'session-data'); + fs.mkdirSync(sessionsDir, { recursive: true }); + + // shortId is derived from the transcript filename UUID (last 8 chars). + const uuid = 'abcdef12-3456-7890-abcd-ef0123456789'; + const shortId = sanitizeSessionId(uuid.slice(-8).toLowerCase()); + const today = getDateString(); + const sessionFile = path.join(sessionsDir, `${today}-${shortId}-session.tmp`); + + // Pre-seed a session file that already has summary markers, so the + // idempotent rewrite path runs .replace() with the new summary block. + fs.writeFileSync( + sessionFile, + `# Session: ${today}\n**Date:** ${today}\n---\n${START}\n## Session Summary\n\n### Tasks\n- old task\n${END}\n` + ); + + // Transcript whose user message contains replacement-special $-sequences. + const userText = 'release $& fallback $$ done'; + const transcript = path.join(home, `${uuid}.jsonl`); + fs.writeFileSync( + transcript, + JSON.stringify({ type: 'user', message: { role: 'user', content: userText } }) + '\n' + ); + + const res = spawnSync('node', [script], { + encoding: 'utf8', + input: JSON.stringify({ transcript_path: transcript }), + env: { ...process.env, HOME: home, USERPROFILE: home, CLAUDE_SESSION_ID: '' }, + timeout: 10000, + }); + assert.strictEqual(res.status || 0, 0, `hook exited ${res.status}: ${res.stderr}`); + + const out = fs.readFileSync(sessionFile, 'utf8'); + // User text must survive verbatim (no $&/$$ interpretation). + assert.ok(out.includes(`- ${userText}`), `expected verbatim user text in:\n${out}`); + // Exactly one marker pair — a $& bug re-injects the matched block, duplicating markers. + assert.strictEqual(countOccurrences(out, START), 1, `START marker should appear once:\n${out}`); + assert.strictEqual(countOccurrences(out, END), 1, `END marker should appear once:\n${out}`); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }) ? passed++ : failed++); + + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); + process.exit(failed > 0 ? 1 : 0); +} + +runTests();