Skip to content

Commit 2d44cc7

Browse files
committed
test: add E2E concurrent session creation test + fix re-check logic
- 11 new tests: lock mechanism (7), sequential calls (3), parallel E2E (1) - E2E test: 5 parallel processes -> exactly 1 session (verified) - Fix: re-check inside lock trusts mapping unconditionally (winner's session may have dead pid if worker exited, but it's fresh by definition) - Export lock functions for testing
1 parent cf777d6 commit 2d44cc7

2 files changed

Lines changed: 226 additions & 48 deletions

File tree

src/storage/sessions.ts

Lines changed: 37 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -490,11 +490,12 @@ const LOCK_STALE_MS = 5_000;
490490
const LOCK_WAIT_MS = 500;
491491
const LOCK_POLL_MS = 50;
492492

493-
function sessionLockPath(projectPath: string, claudeSessionId: string): string {
493+
// Exported for testing
494+
export function sessionLockPath(projectPath: string, claudeSessionId: string): string {
494495
return join(activeSessionsDir(projectPath), `${claudeSessionId}.lock`);
495496
}
496497

497-
function acquireLock(projectPath: string, claudeSessionId: string): boolean {
498+
export function acquireLock(projectPath: string, claudeSessionId: string): boolean {
498499
const lp = sessionLockPath(projectPath, claudeSessionId);
499500
ensureDir(activeSessionsDir(projectPath));
500501
try {
@@ -511,7 +512,7 @@ function acquireLock(projectPath: string, claudeSessionId: string): boolean {
511512
}
512513
}
513514

514-
function releaseLock(projectPath: string, claudeSessionId: string): void {
515+
export function releaseLock(projectPath: string, claudeSessionId: string): void {
515516
try { unlinkSync(sessionLockPath(projectPath, claudeSessionId)); } catch {}
516517
}
517518

@@ -554,6 +555,7 @@ export function ensureAxmeSessionForClaude(
554555
* the existing session id instead of creating a fresh empty-tail session. */
555556
toolName?: string,
556557
): string {
558+
// Fast path: live mapping exists, just reuse it (no lock needed).
557559
const existing = readClaudeSessionMapping(projectPath, claudeSessionId);
558560
if (existing) {
559561
const existingSession = loadSession(projectPath, existing);
@@ -562,69 +564,56 @@ export function ensureAxmeSessionForClaude(
562564
existingSession.auditedAt != null ||
563565
(existingSession.pid != null && !isPidAlive(existingSession.pid));
564566
if (!isStale) {
565-
// Live mapping — attach transcript and reuse.
566567
attachClaudeSession(projectPath, existing, {
567568
id: claudeSessionId,
568569
transcriptPath,
569570
role: "main",
570571
});
571-
// Always refresh ownerPpid — after VS Code reload the Claude Code
572-
// PID changes but the old process may still be alive (different
573-
// window or zombie). The MCP server matches by ownerPpid === OWN_PPID,
574-
// so the mapping must point to the current Claude Code instance.
575572
writeClaudeSessionMapping(projectPath, claudeSessionId, existing);
576573
return existing;
577574
}
578-
// Read-only tools (Read/Glob/Grep) should not create fresh sessions from
579-
// stale mappings — that produces empty "tail" sessions with 0 extractions.
580-
// Return the stale id instead; the next mutation tool will create a fresh one.
575+
// Read-only tools should not create fresh sessions from stale mappings.
581576
const READ_ONLY_TOOLS = ["Read", "Glob", "Grep"];
582-
if (toolName && READ_ONLY_TOOLS.includes(toolName) && existing) {
577+
if (toolName && READ_ONLY_TOOLS.includes(toolName)) {
583578
return existing;
584579
}
585-
// Stale mapping: acquire lock to prevent parallel hooks from each
586-
// creating a new session. Only the lock winner creates; others re-read.
587-
const gotLock = waitForLock(projectPath, claudeSessionId);
588-
try {
589-
// Re-check inside lock — another process may have won the race
590-
const recheck = readClaudeSessionMapping(projectPath, claudeSessionId);
591-
if (recheck && recheck !== existing) {
592-
const recheckSession = loadSession(projectPath, recheck);
593-
if (recheckSession && !recheckSession.auditedAt &&
594-
(recheckSession.pid == null || isPidAlive(recheckSession.pid))) {
595-
attachClaudeSession(projectPath, recheck, { id: claudeSessionId, transcriptPath, role: "main" });
596-
return recheck;
597-
}
598-
}
599-
// We won the race (or lock timed out) — create fresh session
580+
}
581+
582+
// Slow path: need to create a new session (stale mapping OR first time).
583+
// Acquire filesystem lock to prevent parallel hooks from each creating one.
584+
const gotLock = waitForLock(projectPath, claudeSessionId);
585+
try {
586+
// Re-check inside lock — another process may have won the race.
587+
// If a DIFFERENT mapping appeared (created by the lock winner), use it
588+
// unconditionally — the winner just created it, so it's fresh by definition.
589+
// Do NOT re-run stale checks here: the winner's process may have already
590+
// exited (test workers, short-lived hooks), making the pid look dead.
591+
const recheck = readClaudeSessionMapping(projectPath, claudeSessionId);
592+
if (recheck && recheck !== existing) {
593+
attachClaudeSession(projectPath, recheck, { id: claudeSessionId, transcriptPath, role: "main" });
594+
return recheck;
595+
}
596+
// We won the race (or lock timed out) — create fresh session.
597+
if (existing) {
598+
const existingSession = loadSession(projectPath, existing);
600599
process.stderr.write(
601600
`AXME: stale mapping for Claude session ${claudeSessionId} → ` +
602601
`AXME ${existing} (audited=${existingSession?.auditedAt ?? "no"}, ` +
603602
`pid=${existingSession?.pid ?? "?"}). Creating fresh AXME session.\n`,
604603
);
605-
const axmeSession = createSession(projectPath);
606-
try { logSessionStart(projectPath, axmeSession.id); } catch {}
607-
writeClaudeSessionMapping(projectPath, claudeSessionId, axmeSession.id);
608-
attachClaudeSession(projectPath, axmeSession.id, {
609-
id: claudeSessionId,
610-
transcriptPath,
611-
role: "main",
612-
});
613-
return axmeSession.id;
614-
} finally {
615-
if (gotLock) releaseLock(projectPath, claudeSessionId);
616604
}
605+
const axmeSession = createSession(projectPath);
606+
try { logSessionStart(projectPath, axmeSession.id); } catch {}
607+
writeClaudeSessionMapping(projectPath, claudeSessionId, axmeSession.id);
608+
attachClaudeSession(projectPath, axmeSession.id, {
609+
id: claudeSessionId,
610+
transcriptPath,
611+
role: "main",
612+
});
613+
return axmeSession.id;
614+
} finally {
615+
if (gotLock) releaseLock(projectPath, claudeSessionId);
617616
}
618-
// No existing mapping — first time for this Claude session
619-
const axmeSession = createSession(projectPath);
620-
try { logSessionStart(projectPath, axmeSession.id); } catch {}
621-
writeClaudeSessionMapping(projectPath, claudeSessionId, axmeSession.id);
622-
attachClaudeSession(projectPath, axmeSession.id, {
623-
id: claudeSessionId,
624-
transcriptPath,
625-
role: "main",
626-
});
627-
return axmeSession.id;
628617
}
629618

630619
// --- Legacy single-file API (DEPRECATED, kept for backward compatibility) ---

test/audit-dedup.test.ts

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
import { describe, it, beforeEach, afterEach } from "node:test";
2+
import assert from "node:assert/strict";
3+
import { mkdirSync, rmSync, existsSync, writeFileSync, utimesSync, readdirSync } from "node:fs";
4+
import { join } from "node:path";
5+
import { spawn } from "node:child_process";
6+
import {
7+
acquireLock,
8+
releaseLock,
9+
sessionLockPath,
10+
ensureAxmeSessionForClaude,
11+
listSessions,
12+
} from "../src/storage/sessions.js";
13+
14+
const TEST_ROOT = "/tmp/axme-audit-dedup-test";
15+
const AXME_DIR = join(TEST_ROOT, ".axme-code");
16+
17+
function setup() {
18+
rmSync(TEST_ROOT, { recursive: true, force: true });
19+
mkdirSync(join(AXME_DIR, "sessions"), { recursive: true });
20+
mkdirSync(join(AXME_DIR, "active-sessions"), { recursive: true });
21+
}
22+
23+
function cleanup() {
24+
rmSync(TEST_ROOT, { recursive: true, force: true });
25+
}
26+
27+
// ===== Lock mechanism =====
28+
29+
describe("acquireLock / releaseLock", () => {
30+
beforeEach(() => setup());
31+
afterEach(() => cleanup());
32+
33+
it("acquires lock on first call", () => {
34+
const got = acquireLock(TEST_ROOT, "test-session-1");
35+
assert.equal(got, true);
36+
assert.ok(existsSync(sessionLockPath(TEST_ROOT, "test-session-1")));
37+
releaseLock(TEST_ROOT, "test-session-1");
38+
});
39+
40+
it("fails on second concurrent acquire", () => {
41+
const got1 = acquireLock(TEST_ROOT, "test-session-2");
42+
assert.equal(got1, true);
43+
const got2 = acquireLock(TEST_ROOT, "test-session-2");
44+
assert.equal(got2, false);
45+
releaseLock(TEST_ROOT, "test-session-2");
46+
});
47+
48+
it("succeeds after release", () => {
49+
acquireLock(TEST_ROOT, "test-session-3");
50+
releaseLock(TEST_ROOT, "test-session-3");
51+
const got = acquireLock(TEST_ROOT, "test-session-3");
52+
assert.equal(got, true);
53+
releaseLock(TEST_ROOT, "test-session-3");
54+
});
55+
56+
it("lock file is cleaned up after release", () => {
57+
acquireLock(TEST_ROOT, "test-session-4");
58+
releaseLock(TEST_ROOT, "test-session-4");
59+
assert.ok(!existsSync(sessionLockPath(TEST_ROOT, "test-session-4")));
60+
});
61+
62+
it("cleans stale lock (>5s old)", () => {
63+
// Create a lock file and backdate it
64+
const lp = sessionLockPath(TEST_ROOT, "test-session-5");
65+
mkdirSync(join(AXME_DIR, "active-sessions"), { recursive: true });
66+
writeFileSync(lp, "");
67+
const old = new Date(Date.now() - 10_000); // 10 seconds ago
68+
utimesSync(lp, old, old);
69+
70+
// Should succeed because lock is stale
71+
const got = acquireLock(TEST_ROOT, "test-session-5");
72+
assert.equal(got, true);
73+
releaseLock(TEST_ROOT, "test-session-5");
74+
});
75+
76+
it("does NOT clean fresh lock (<5s old)", () => {
77+
// Create a lock file that is fresh
78+
const lp = sessionLockPath(TEST_ROOT, "test-session-6");
79+
mkdirSync(join(AXME_DIR, "active-sessions"), { recursive: true });
80+
writeFileSync(lp, "");
81+
// Don't backdate - it's fresh
82+
83+
const got = acquireLock(TEST_ROOT, "test-session-6");
84+
assert.equal(got, false); // should fail - lock is fresh, held by someone
85+
rmSync(lp, { force: true });
86+
});
87+
88+
it("different session IDs get independent locks", () => {
89+
const got1 = acquireLock(TEST_ROOT, "session-a");
90+
const got2 = acquireLock(TEST_ROOT, "session-b");
91+
assert.equal(got1, true);
92+
assert.equal(got2, true);
93+
releaseLock(TEST_ROOT, "session-a");
94+
releaseLock(TEST_ROOT, "session-b");
95+
});
96+
});
97+
98+
// ===== Concurrent session creation (E2E) =====
99+
100+
describe("ensureAxmeSessionForClaude - concurrent calls", () => {
101+
beforeEach(() => setup());
102+
afterEach(() => cleanup());
103+
104+
it("single call creates exactly one session", () => {
105+
const id = ensureAxmeSessionForClaude(TEST_ROOT, "claude-1", "/tmp/transcript.jsonl");
106+
assert.ok(id);
107+
assert.ok(existsSync(join(AXME_DIR, "sessions", id, "meta.json")));
108+
const sessions = readdirSync(join(AXME_DIR, "sessions"));
109+
assert.equal(sessions.length, 1);
110+
});
111+
112+
it("two sequential calls with same Claude ID return same session", () => {
113+
const id1 = ensureAxmeSessionForClaude(TEST_ROOT, "claude-2", "/tmp/t.jsonl");
114+
const id2 = ensureAxmeSessionForClaude(TEST_ROOT, "claude-2", "/tmp/t.jsonl");
115+
assert.equal(id1, id2);
116+
});
117+
118+
it("different Claude IDs create different sessions", () => {
119+
const id1 = ensureAxmeSessionForClaude(TEST_ROOT, "claude-a", "/tmp/t.jsonl");
120+
const id2 = ensureAxmeSessionForClaude(TEST_ROOT, "claude-b", "/tmp/t.jsonl");
121+
assert.notEqual(id1, id2);
122+
});
123+
});
124+
125+
// ===== Concurrent process E2E =====
126+
// This is the real test: spawn N child processes that each call
127+
// ensureAxmeSessionForClaude concurrently, simulating parallel hooks.
128+
129+
describe("ensureAxmeSessionForClaude - parallel processes (E2E)", () => {
130+
beforeEach(() => setup());
131+
afterEach(() => cleanup());
132+
133+
it("5 parallel processes create exactly 1 session", async () => {
134+
const N = 5;
135+
const claudeId = "concurrent-test-claude";
136+
const transcript = "/tmp/test-transcript.jsonl";
137+
138+
// Write a worker script
139+
const workerScript = join(TEST_ROOT, "worker.ts");
140+
writeFileSync(workerScript, [
141+
`import { ensureAxmeSessionForClaude } from "${join(process.cwd(), "src/storage/sessions.js")}";`,
142+
`const result = ensureAxmeSessionForClaude("${TEST_ROOT}", "${claudeId}", "${transcript}");`,
143+
`process.stdout.write(result);`,
144+
].join("\n"));
145+
146+
// Spawn N workers in parallel using npx tsx
147+
const runWorker = () =>
148+
new Promise<string>((resolve, reject) => {
149+
const child = spawn("npx", ["tsx", workerScript], {
150+
stdio: ["pipe", "pipe", "pipe"],
151+
cwd: process.cwd(),
152+
});
153+
let stdout = "";
154+
let stderr = "";
155+
child.stdout.on("data", (d: Buffer) => { stdout += d.toString(); });
156+
child.stderr.on("data", (d: Buffer) => { stderr += d.toString(); });
157+
child.on("exit", (code) => {
158+
if (code === 0) resolve(stdout.trim());
159+
else reject(new Error(`worker exited ${code}: ${stderr.slice(0, 200)}`));
160+
});
161+
child.on("error", reject);
162+
});
163+
164+
const results = await Promise.allSettled(Array.from({ length: N }, runWorker));
165+
const successes = results.filter(r => r.status === "fulfilled") as PromiseFulfilledResult<string>[];
166+
const failures = results.filter(r => r.status === "rejected") as PromiseRejectedResult[];
167+
if (failures.length > 0) {
168+
process.stderr.write(` Worker failures (${failures.length}): ${failures.map(f => f.reason?.message?.slice(0, 100)).join("; ")}\n`);
169+
}
170+
171+
// At least 1 should succeed
172+
assert.ok(successes.length >= 1, `at least 1 worker should succeed, got ${successes.length}`);
173+
174+
// Count actual sessions created - should be exactly 1
175+
const sessions = readdirSync(join(AXME_DIR, "sessions"));
176+
console.log(` ${N} parallel workers -> ${sessions.length} sessions, ${successes.length} succeeded`);
177+
assert.equal(sessions.length, 1,
178+
`expected exactly 1 session from ${N} concurrent processes, got ${sessions.length}: ${sessions.join(", ")}`);
179+
180+
// All successful workers should return the same session ID
181+
const uniqueIds = new Set(successes.map(r => r.value));
182+
assert.equal(uniqueIds.size, 1,
183+
`all workers should return same session ID, got ${[...uniqueIds].join(", ")}`);
184+
185+
// No stale lock files
186+
const lockFiles = readdirSync(join(AXME_DIR, "active-sessions")).filter(f => f.endsWith(".lock"));
187+
assert.equal(lockFiles.length, 0, `no stale lock files, found: ${lockFiles.join(", ")}`);
188+
});
189+
});

0 commit comments

Comments
 (0)