6577c04a1f
Closes the remaining parallel-session-lock remarks on top of the keying fix
(7a469dc9), with NO weakening of same-worktree serialization:
- D: the block message now identifies the holder by its STABLE session_id and
marks the recorded pid as transient ("may change between attempts"). Chasing
the pid is what led to closing the wrong session. Decision logic is unchanged
(text only) — existing /pid N/ triage assertion still holds.
- B: pruneStaleLocks() best-effort deletes leaked lock files that are ALREADY
stale by the shared isStale() definition (now exported from the pure module —
single source of truth). Active within-TTL locks are never touched, so the
serialization guarantee is not weakened. Wired into the PreToolUse branch of
main(), wrapped so hygiene can never break the gate (fail-open).
- C (no code): release-on-SessionEnd needs only a settings.json registration
(owner action) — the existing !tool_name branch already releases. Documented
in the plan. Until then, leaked locks self-heal via B + the 5-min TTL takeover.
TDD: RED -> GREEN per behavior. tools-vitest 2014 passed / 2 skipped.
Backlog items B/C/D; plan docs/superpowers/plans/2026-05-31-discipline-guard-backlog.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
252 lines
9.7 KiB
JavaScript
252 lines
9.7 KiB
JavaScript
// tools/enforce-parallel-session-lock.test.mjs
|
|
// Stream H Task 7 — wrapper tests around the pure parallel-session-lock module.
|
|
import { describe, it, expect } from 'vitest';
|
|
import { decide } from './enforce-parallel-session-lock.mjs';
|
|
|
|
describe('enforce-parallel-session-lock wrapper (Stream H Task 7)', () => {
|
|
it('allow when acquire succeeded (fresh own-lock)', () => {
|
|
const r = decide({
|
|
acquireResult: { acquired: true, holder: { session_id: 's1', pid: 100, acquired_at: 1000 } },
|
|
sessionId: 's1',
|
|
});
|
|
expect(r.block).toBe(false);
|
|
});
|
|
|
|
it('block when another session holds the lock', () => {
|
|
const r = decide({
|
|
acquireResult: { acquired: false, holder: { session_id: 'other-session', pid: 999, acquired_at: 500 } },
|
|
sessionId: 's1',
|
|
});
|
|
expect(r.block).toBe(true);
|
|
expect(r.reason).toMatch(/parallel session lock.*other-session/i);
|
|
});
|
|
|
|
it('allow when same-session re-acquires (takeover)', () => {
|
|
const r = decide({
|
|
acquireResult: { acquired: true, holder: { session_id: 's1', pid: 100, acquired_at: 2000 } },
|
|
sessionId: 's1',
|
|
});
|
|
expect(r.block).toBe(false);
|
|
});
|
|
|
|
it('fail-open when acquireResult is missing (internal error path)', () => {
|
|
expect(decide({ acquireResult: null, sessionId: 's1' }).block).toBe(false);
|
|
expect(decide({ acquireResult: undefined, sessionId: 's1' }).block).toBe(false);
|
|
});
|
|
|
|
it('block message identifies the other holder pid for human triage', () => {
|
|
const r = decide({
|
|
acquireResult: { acquired: false, holder: { session_id: 'other', pid: 42, acquired_at: 0 } },
|
|
sessionId: 's1',
|
|
});
|
|
expect(r.reason).toMatch(/pid 42/);
|
|
});
|
|
});
|
|
|
|
// D (2026-05-31): the block message must steer the human to the STABLE identity
|
|
// (session id), not the transient hook pid — chasing the pid was what caused the
|
|
// owner to close the wrong session and deadlock the workspace.
|
|
describe('decide() message clarity (D) — pid is transient, identify by session id', () => {
|
|
const blocked = { acquired: false, holder: { session_id: 'sess-A', pid: 12552, acquired_at: 0 } };
|
|
|
|
it('names the holder session id as the stable identity', () => {
|
|
expect(decide({ acquireResult: blocked, sessionId: 's1' }).reason).toMatch(/sess-A/);
|
|
});
|
|
|
|
it('marks the pid as changeable so the human does not chase it', () => {
|
|
expect(decide({ acquireResult: blocked, sessionId: 's1' }).reason).toMatch(/may change|transient/i);
|
|
});
|
|
|
|
it('still surfaces the pid for triage', () => {
|
|
expect(decide({ acquireResult: blocked, sessionId: 's1' }).reason).toMatch(/12552/);
|
|
});
|
|
});
|
|
|
|
// Live wiring (point 2, 2026-05-31): PreToolUse acquires/refreshes the lock,
|
|
// Stop releases it. I/O is injected (readLock/writeLock/deleteLock) so the
|
|
// wiring stays pure and unit-testable; main() binds real fs.
|
|
import { runAcquireDecision, runReleaseAction } from './enforce-parallel-session-lock.mjs';
|
|
|
|
describe('runAcquireDecision — PreToolUse acquire/refresh wiring', () => {
|
|
it('allows and writes a fresh lock when none exists', () => {
|
|
let written = null;
|
|
const r = runAcquireDecision({
|
|
event: { tool_name: 'Edit', session_id: 'S1' },
|
|
now: 1000, pid: 42, cwd: '/ws',
|
|
readLock: () => null,
|
|
writeLock: (rec) => { written = rec; },
|
|
});
|
|
expect(r.block).toBe(false);
|
|
expect(written).toMatchObject({ session_id: 'S1', pid: 42, acquired_at: 1000 });
|
|
});
|
|
|
|
it('blocks when another session holds a fresh lock', () => {
|
|
const r = runAcquireDecision({
|
|
event: { tool_name: 'Edit', session_id: 'S2' },
|
|
now: 1000, pid: 7, cwd: '/ws',
|
|
readLock: () => ({ schema_version: 1, session_id: 'S1', pid: 99, acquired_at: 900, ttl_ms: 300000 }),
|
|
writeLock: () => {},
|
|
});
|
|
expect(r.block).toBe(true);
|
|
expect(r.reason).toMatch(/S1|pid 99|parallel session/i);
|
|
});
|
|
|
|
it('allows (refresh) when the same session already holds the lock', () => {
|
|
let written = null;
|
|
const r = runAcquireDecision({
|
|
event: { tool_name: 'Edit', session_id: 'S1' },
|
|
now: 2000, pid: 42, cwd: '/ws',
|
|
readLock: () => ({ schema_version: 1, session_id: 'S1', pid: 42, acquired_at: 900, ttl_ms: 300000 }),
|
|
writeLock: (rec) => { written = rec; },
|
|
});
|
|
expect(r.block).toBe(false);
|
|
expect(written.acquired_at).toBe(2000);
|
|
});
|
|
|
|
it('takes over a stale lock from another session (TTL expired)', () => {
|
|
let written = null;
|
|
const r = runAcquireDecision({
|
|
event: { tool_name: 'Edit', session_id: 'S2' },
|
|
now: 1_000_000, pid: 7, cwd: '/ws',
|
|
readLock: () => ({ schema_version: 1, session_id: 'S1', pid: 99, acquired_at: 0, ttl_ms: 300000 }),
|
|
writeLock: (rec) => { written = rec; },
|
|
});
|
|
expect(r.block).toBe(false);
|
|
expect(written.session_id).toBe('S2');
|
|
});
|
|
});
|
|
|
|
describe('runReleaseAction — Stop release wiring', () => {
|
|
it('deletes the lock when this session owns it', () => {
|
|
let deleted = false;
|
|
runReleaseAction({
|
|
event: { session_id: 'S1' },
|
|
cwd: '/ws',
|
|
readLock: () => ({ schema_version: 1, session_id: 'S1', pid: 42, acquired_at: 0, ttl_ms: 300000 }),
|
|
deleteLock: () => { deleted = true; },
|
|
});
|
|
expect(deleted).toBe(true);
|
|
});
|
|
|
|
it('does NOT delete a lock owned by another session', () => {
|
|
let deleted = false;
|
|
runReleaseAction({
|
|
event: { session_id: 'S2' },
|
|
cwd: '/ws',
|
|
readLock: () => ({ schema_version: 1, session_id: 'S1', pid: 42, acquired_at: 0, ttl_ms: 300000 }),
|
|
deleteLock: () => { deleted = true; },
|
|
});
|
|
expect(deleted).toBe(false);
|
|
});
|
|
|
|
it('is a no-op when no lock file exists', () => {
|
|
let deleted = false;
|
|
runReleaseAction({
|
|
event: { session_id: 'S1' },
|
|
cwd: '/ws',
|
|
readLock: () => null,
|
|
deleteLock: () => { deleted = true; },
|
|
});
|
|
expect(deleted).toBe(false);
|
|
});
|
|
});
|
|
|
|
// Cross-worktree false-block fix (2026-05-31). The lock must key on the session's
|
|
// stable work-tree root (from event.cwd → git toplevel), NOT the hook process.cwd()
|
|
// — which collapses to the main repo dir after a session resume, making sessions in
|
|
// DIFFERENT worktrees share one lock and block each other.
|
|
import { resolveWorkspacePath, pruneStaleLocks } from './enforce-parallel-session-lock.mjs';
|
|
|
|
describe('resolveWorkspacePath — stable worktree key', () => {
|
|
it('keys on event.cwd (the session worktree), not the hook process.cwd()', () => {
|
|
const r = resolveWorkspacePath({
|
|
event: { cwd: '/repo/.claude/worktrees/wt-A' },
|
|
processCwd: '/repo',
|
|
runGitToplevel: (dir) => dir,
|
|
});
|
|
expect(r).toBe('/repo/.claude/worktrees/wt-A');
|
|
});
|
|
|
|
it('gives different keys for two different worktrees (no cross-block)', () => {
|
|
const opts = { processCwd: '/repo', runGitToplevel: (dir) => dir };
|
|
const a = resolveWorkspacePath({ event: { cwd: '/repo/.claude/worktrees/wt-A' }, ...opts });
|
|
const b = resolveWorkspacePath({ event: { cwd: '/repo/.claude/worktrees/wt-B' }, ...opts });
|
|
expect(a).not.toBe(b);
|
|
});
|
|
|
|
it('resolves to the git work-tree root (collapses subdir variance)', () => {
|
|
const r = resolveWorkspacePath({
|
|
event: { cwd: '/repo/.claude/worktrees/wt-A/tools' },
|
|
processCwd: '/repo',
|
|
runGitToplevel: () => '/repo/.claude/worktrees/wt-A',
|
|
});
|
|
expect(r).toBe('/repo/.claude/worktrees/wt-A');
|
|
});
|
|
|
|
it('falls back to processCwd when event.cwd is absent', () => {
|
|
const r = resolveWorkspacePath({
|
|
event: { tool_name: 'Edit' },
|
|
processCwd: '/repo',
|
|
runGitToplevel: (dir) => dir,
|
|
});
|
|
expect(r).toBe('/repo');
|
|
});
|
|
|
|
it('falls back to the raw dir when git toplevel resolution fails (fail-open)', () => {
|
|
const r = resolveWorkspacePath({
|
|
event: { cwd: '/some/dir' },
|
|
processCwd: '/repo',
|
|
runGitToplevel: () => '',
|
|
});
|
|
expect(r).toBe('/some/dir');
|
|
});
|
|
});
|
|
|
|
// B (2026-05-31): disk hygiene. Leaked lock files (session closed without a clean
|
|
// Stop) pile up in ~/.claude/runtime. Pruning ONLY removes records that are
|
|
// already stale by the SAME isStale() definition acquire() uses — so it can never
|
|
// drop an active (within-TTL) lock and never weakens same-worktree serialization.
|
|
describe('pruneStaleLocks — drops only already-stale leaked locks (B)', () => {
|
|
const fresh = { schema_version: 1, session_id: 'A', pid: 1, acquired_at: 1000, ttl_ms: 300000 };
|
|
const stale = { schema_version: 1, session_id: 'B', pid: 2, acquired_at: 0, ttl_ms: 100 };
|
|
const isStaleFn = (rec, now) => !rec || (now - (rec && rec.acquired_at || 0)) > ((rec && rec.ttl_ms) || 300000);
|
|
|
|
it('deletes stale lock files and never the fresh (active) ones', () => {
|
|
const records = { '/r/lock-fresh.json': fresh, '/r/lock-stale.json': stale };
|
|
const deleted = [];
|
|
const r = pruneStaleLocks({
|
|
files: Object.keys(records),
|
|
readRecord: (f) => records[f],
|
|
deleteRecord: (f) => deleted.push(f),
|
|
isStaleFn, now: 1000,
|
|
});
|
|
expect(deleted).toEqual(['/r/lock-stale.json']);
|
|
expect(r.pruned).toBe(1);
|
|
});
|
|
|
|
it('treats an unreadable/garbage lock file as stale and prunes it', () => {
|
|
const deleted = [];
|
|
pruneStaleLocks({
|
|
files: ['/r/garbage.json'],
|
|
readRecord: () => { throw new Error('bad json'); },
|
|
deleteRecord: (f) => deleted.push(f),
|
|
isStaleFn, now: 1000,
|
|
});
|
|
expect(deleted).toEqual(['/r/garbage.json']);
|
|
});
|
|
|
|
it('never throws when a delete fails (best-effort hygiene)', () => {
|
|
expect(() => pruneStaleLocks({
|
|
files: ['/r/x.json'],
|
|
readRecord: () => stale,
|
|
deleteRecord: () => { throw new Error('locked'); },
|
|
isStaleFn, now: 1000,
|
|
})).not.toThrow();
|
|
});
|
|
|
|
it('does nothing for an empty file list', () => {
|
|
const r = pruneStaleLocks({ files: [], readRecord: () => null, deleteRecord: () => {}, isStaleFn, now: 1 });
|
|
expect(r.pruned).toBe(0);
|
|
});
|
|
});
|