mirror of
https://github.com/affaan-m/everything-claude-code.git
synced 2026-04-22 18:13:41 +08:00
review: address P1/P2 bot feedback on shortId derivation
- Use last-8 chars of transcript UUID instead of first-8, matching getSessionIdShort()'s .slice(-8) convention. Same session now produces the same filename whether shortId comes from CLAUDE_SESSION_ID or transcript_path, so existing .tmp files are not orphaned on upgrade. - Normalize extracted hex prefix to lowercase to avoid case-driven filename divergence from sanitizeSessionId()'s lowercase output. - Explicitly clear CLAUDE_SESSION_ID in the first regression test so the env leak from parent test runs cannot hide the fallback path. - Add regression tests for the lowercase-normalization path and for the case where CLAUDE_SESSION_ID and transcript_path refer to the same UUID (backward compat guarantee). Refs #1494
This commit is contained in:
@@ -190,14 +190,19 @@ async function main() {
|
||||
|
||||
const sessionsDir = getSessionsDir();
|
||||
const today = getDateString();
|
||||
// Prefer the real session UUID (first 8 chars) from transcript_path when available.
|
||||
// Derive shortId from transcript_path UUID when available, using the SAME
|
||||
// last-8-chars convention as getSessionIdShort(sessionId.slice(-8)). This keeps
|
||||
// backward compatibility for normal sessions (the derived shortId matches what
|
||||
// getSessionIdShort() would have produced from the same UUID), while making
|
||||
// every session map to a unique filename based on its own transcript UUID.
|
||||
//
|
||||
// Without this, a parent session and any `claude -p ...` subprocess spawned by
|
||||
// another Stop-hook share the project-name fallback filename, and the subprocess
|
||||
// another Stop hook share the project-name fallback filename, and the subprocess
|
||||
// overwrites the parent's summary. See issue #1494 for full repro details.
|
||||
let shortId = null;
|
||||
if (transcriptPath) {
|
||||
const m = path.basename(transcriptPath).match(/([0-9a-f]{8})-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.jsonl$/i);
|
||||
if (m) { shortId = m[1]; }
|
||||
const m = path.basename(transcriptPath).match(/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\.jsonl$/i);
|
||||
if (m) { shortId = m[1].slice(-8).toLowerCase(); }
|
||||
}
|
||||
if (!shortId) { shortId = getSessionIdShort(); }
|
||||
const sessionFile = path.join(sessionsDir, `${today}-${shortId}-session.tmp`);
|
||||
|
||||
@@ -633,12 +633,14 @@ async function runTests() {
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
// Regression test for #1494: transcript_path UUID takes precedence over fallback
|
||||
// Regression test for #1494: transcript_path UUID-derived shortId (last 8 chars)
|
||||
// isolates sibling subprocess invocations while preserving getSessionIdShort()
|
||||
// backward compatibility (same `.slice(-8)` convention).
|
||||
if (
|
||||
await asyncTest('derives shortId from transcript_path UUID when available', async () => {
|
||||
const isoHome = path.join(os.tmpdir(), `ecc-session-transcript-${Date.now()}`);
|
||||
const transcriptUuid = 'abcdef12-3456-4789-a012-bcdef3456789';
|
||||
const expectedShortId = 'abcdef12'; // First 8 chars of UUID
|
||||
const expectedShortId = 'f3456789'; // Last 8 chars of UUID (matches getSessionIdShort convention)
|
||||
const transcriptPath = path.join(isoHome, 'transcripts', `${transcriptUuid}.jsonl`);
|
||||
|
||||
try {
|
||||
@@ -649,8 +651,9 @@ async function runTests() {
|
||||
await runScript(path.join(scriptsDir, 'session-end.js'), stdinJson, {
|
||||
HOME: isoHome,
|
||||
USERPROFILE: isoHome,
|
||||
// CLAUDE_SESSION_ID intentionally unset so that without the fix the project-name
|
||||
// fallback would be used, exposing the filename collision described in #1494.
|
||||
// Explicitly clear CLAUDE_SESSION_ID so parent env does not leak in and
|
||||
// force the getSessionIdShort() fallback instead of the transcript path.
|
||||
CLAUDE_SESSION_ID: ''
|
||||
});
|
||||
|
||||
const sessionsDir = getCanonicalSessionsDir(isoHome);
|
||||
@@ -667,6 +670,75 @@ async function runTests() {
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
// Regression test for #1494: uppercase UUID hex digits should be normalized to
|
||||
// lowercase so the filename is consistent with getSessionIdShort()'s output.
|
||||
if (
|
||||
await asyncTest('normalizes transcript UUID shortId to lowercase', async () => {
|
||||
const isoHome = path.join(os.tmpdir(), `ecc-session-transcript-upper-${Date.now()}`);
|
||||
const transcriptUuid = 'ABCDEF12-3456-4789-A012-BCDEF3456789';
|
||||
const expectedShortId = 'f3456789'; // last 8 lowercased
|
||||
const transcriptPath = path.join(isoHome, 'transcripts', `${transcriptUuid}.jsonl`);
|
||||
|
||||
try {
|
||||
fs.mkdirSync(path.dirname(transcriptPath), { recursive: true });
|
||||
fs.writeFileSync(transcriptPath, '');
|
||||
|
||||
const stdinJson = JSON.stringify({ transcript_path: transcriptPath });
|
||||
await runScript(path.join(scriptsDir, 'session-end.js'), stdinJson, {
|
||||
HOME: isoHome,
|
||||
USERPROFILE: isoHome,
|
||||
CLAUDE_SESSION_ID: ''
|
||||
});
|
||||
|
||||
const sessionsDir = getCanonicalSessionsDir(isoHome);
|
||||
const now = new Date();
|
||||
const today = `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String(now.getDate()).padStart(2, '0')}`;
|
||||
const sessionFile = path.join(sessionsDir, `${today}-${expectedShortId}-session.tmp`);
|
||||
|
||||
assert.ok(fs.existsSync(sessionFile), `Session file with lowercase shortId should exist: ${sessionFile}`);
|
||||
} finally {
|
||||
fs.rmSync(isoHome, { recursive: true, force: true });
|
||||
}
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
// Regression test for #1494: when CLAUDE_SESSION_ID and transcript_path refer to the
|
||||
// same UUID, the derived shortId must be identical to the pre-fix behaviour so that
|
||||
// existing .tmp files are not orphaned on upgrade.
|
||||
if (
|
||||
await asyncTest('matches getSessionIdShort when transcript UUID equals CLAUDE_SESSION_ID', async () => {
|
||||
const isoHome = path.join(os.tmpdir(), `ecc-session-transcript-match-${Date.now()}`);
|
||||
const sessionUuid = '11223344-5566-4778-8899-aabbccddeeff';
|
||||
const expectedShortId = 'ccddeeff'; // last 8 chars of both transcript UUID and CLAUDE_SESSION_ID
|
||||
const transcriptPath = path.join(isoHome, 'transcripts', `${sessionUuid}.jsonl`);
|
||||
|
||||
try {
|
||||
fs.mkdirSync(path.dirname(transcriptPath), { recursive: true });
|
||||
fs.writeFileSync(transcriptPath, '');
|
||||
|
||||
const stdinJson = JSON.stringify({ transcript_path: transcriptPath });
|
||||
await runScript(path.join(scriptsDir, 'session-end.js'), stdinJson, {
|
||||
HOME: isoHome,
|
||||
USERPROFILE: isoHome,
|
||||
CLAUDE_SESSION_ID: sessionUuid
|
||||
});
|
||||
|
||||
const sessionsDir = getCanonicalSessionsDir(isoHome);
|
||||
const now = new Date();
|
||||
const today = `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String(now.getDate()).padStart(2, '0')}`;
|
||||
const sessionFile = path.join(sessionsDir, `${today}-${expectedShortId}-session.tmp`);
|
||||
|
||||
assert.ok(fs.existsSync(sessionFile), `Session filename should match the pre-fix CLAUDE_SESSION_ID-based name: ${sessionFile}`);
|
||||
} finally {
|
||||
fs.rmSync(isoHome, { recursive: true, force: true });
|
||||
}
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await asyncTest('writes project, branch, and worktree metadata into new session files', async () => {
|
||||
const isoHome = path.join(os.tmpdir(), `ecc-session-metadata-${Date.now()}`);
|
||||
|
||||
Reference in New Issue
Block a user