diff --git a/gitnexus-claude-plugin/hooks/gitnexus-hook.js b/gitnexus-claude-plugin/hooks/gitnexus-hook.js index 577e7f16c..35945df04 100644 --- a/gitnexus-claude-plugin/hooks/gitnexus-hook.js +++ b/gitnexus-claude-plugin/hooks/gitnexus-hook.js @@ -193,8 +193,11 @@ function acquireHookSlot(gitNexusDir) { try { fs.mkdirSync(lockDir, { recursive: true }); } catch { - // Cannot create lock dir — fall through unguarded so the hook still works. - return () => {}; + // Cannot create lock dir (read-only fs, cross-user perm denial, out of + // inodes, etc.) — fail closed by returning null. Caller skips augment. + // Fail-open here would let N concurrent hooks all proceed unguarded and + // reintroduce the #1486 fan-out the guard exists to prevent. + return null; } const myPidStr = String(process.pid); @@ -269,9 +272,11 @@ function acquireHookSlot(gitNexusDir) { /* already closed */ } } - // PID-liveness wins over age (avoids evicting a slow-but-alive hook). - // Age check is a safety net against PID reuse on long-abandoned slots: - // 30s >> the 7s augment timeout, so a healthy run never hits it. + // For slots younger than HOOK_LOCK_STALE_MS, PID-liveness wins — + // a slow-but-alive hook is never wrongly evicted. For older slots, + // age is the final arbiter as a defense against PID reuse on long- + // abandoned slots. 30s >> the 7s augment timeout, so a healthy run + // never crosses this threshold. if (isLive && Date.now() - mtimeMs > HOOK_LOCK_STALE_MS) { isLive = false; } diff --git a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs index d4c3035fb..010e86157 100644 --- a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs +++ b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs @@ -223,8 +223,11 @@ function acquireHookSlot(gitNexusDir) { try { fs.mkdirSync(lockDir, { recursive: true }); } catch { - // Cannot create lock dir — fall through unguarded so the hook still works. - return () => {}; + // Cannot create lock dir (read-only fs, cross-user perm denial, out of + // inodes, etc.) — fail closed by returning null. Caller skips augment. + // Fail-open here would let N concurrent hooks all proceed unguarded and + // reintroduce the #1486 fan-out the guard exists to prevent. + return null; } const myPidStr = String(process.pid); @@ -299,9 +302,11 @@ function acquireHookSlot(gitNexusDir) { /* already closed */ } } - // PID-liveness wins over age (avoids evicting a slow-but-alive hook). - // Age check is a safety net against PID reuse on long-abandoned slots: - // 30s >> the 7s augment timeout, so a healthy run never hits it. + // For slots younger than HOOK_LOCK_STALE_MS, PID-liveness wins — + // a slow-but-alive hook is never wrongly evicted. For older slots, + // age is the final arbiter as a defense against PID reuse on long- + // abandoned slots. 30s >> the 7s augment timeout, so a healthy run + // never crosses this threshold. if (isLive && Date.now() - mtimeMs > HOOK_LOCK_STALE_MS) { isLive = false; } diff --git a/gitnexus/hooks/claude/gitnexus-hook.cjs b/gitnexus/hooks/claude/gitnexus-hook.cjs index 23baf9af1..446b758af 100755 --- a/gitnexus/hooks/claude/gitnexus-hook.cjs +++ b/gitnexus/hooks/claude/gitnexus-hook.cjs @@ -193,8 +193,11 @@ function acquireHookSlot(gitNexusDir) { try { fs.mkdirSync(lockDir, { recursive: true }); } catch { - // Cannot create lock dir — fall through unguarded so the hook still works. - return () => {}; + // Cannot create lock dir (read-only fs, cross-user perm denial, out of + // inodes, etc.) — fail closed by returning null. Caller skips augment. + // Fail-open here would let N concurrent hooks all proceed unguarded and + // reintroduce the #1486 fan-out the guard exists to prevent. + return null; } const myPidStr = String(process.pid); @@ -269,9 +272,11 @@ function acquireHookSlot(gitNexusDir) { /* already closed */ } } - // PID-liveness wins over age (avoids evicting a slow-but-alive hook). - // Age check is a safety net against PID reuse on long-abandoned slots: - // 30s >> the 7s augment timeout, so a healthy run never hits it. + // For slots younger than HOOK_LOCK_STALE_MS, PID-liveness wins — + // a slow-but-alive hook is never wrongly evicted. For older slots, + // age is the final arbiter as a defense against PID reuse on long- + // abandoned slots. 30s >> the 7s augment timeout, so a healthy run + // never crosses this threshold. if (isLive && Date.now() - mtimeMs > HOOK_LOCK_STALE_MS) { isLive = false; } diff --git a/gitnexus/test/unit/cursor-hook.test.ts b/gitnexus/test/unit/cursor-hook.test.ts index 329095c92..9d484adf1 100644 --- a/gitnexus/test/unit/cursor-hook.test.ts +++ b/gitnexus/test/unit/cursor-hook.test.ts @@ -420,6 +420,29 @@ describe('Cursor hook concurrency guard', () => { ); expect(slotFn).not.toContain('readdirSync'); }); + + it('fails closed when lock dir cannot be created', () => { + // Regression: see hooks.test.ts. The mkdirSync catch must return null + // (skip augment) rather than `() => {}` (proceed unguarded), so that + // a read-only or cross-user `.gitnexus/` cannot reintroduce #1486. + const slotFn = source.slice( + source.indexOf('function acquireHookSlot'), + source.indexOf('function', source.indexOf('function acquireHookSlot') + 1), + ); + const mkdirCatch = slotFn.slice( + slotFn.indexOf('fs.mkdirSync(lockDir'), + slotFn.indexOf('const myPidStr'), + ); + expect(mkdirCatch).toContain('return null'); + expect(mkdirCatch).not.toMatch(/return\s*\(\s*\)\s*=>\s*\{\s*\}/); + }); + + // Note: the 10-concurrent-spawner burst test that validates `wx` + // (O_CREAT|O_EXCL) under simultaneous contention lives in + // hooks.test.ts. The Cursor hook uses byte-for-byte the same + // acquireHookSlot, so duplicating the burst test here would only test + // the OS primitive, not Cursor-specific wiring. The source-level checks + // above guarantee the Cursor hook keeps calling that same algorithm. }); // ─── Integration: concurrency guard skips when slots are full ────── diff --git a/gitnexus/test/unit/hooks.test.ts b/gitnexus/test/unit/hooks.test.ts index c34667367..c3ca2a4a5 100644 --- a/gitnexus/test/unit/hooks.test.ts +++ b/gitnexus/test/unit/hooks.test.ts @@ -331,6 +331,24 @@ describe('PreToolUse concurrency guard', () => { ); expect(slotFn).not.toContain('readdirSync'); }); + + it(`${label} hook fails closed when lock dir cannot be created`, () => { + // Regression: an earlier revision returned `() => {}` (truthy no-op) on + // mkdirSync failure, which left callers — `if (!release) return;` — to + // proceed unguarded and reintroduce the #1486 fan-out on read-only or + // cross-user `.gitnexus/` setups. The guard must fail closed (null). + const source = fs.readFileSync(hookPath, 'utf-8'); + const slotFn = source.slice( + source.indexOf('function acquireHookSlot'), + source.indexOf('function', source.indexOf('function acquireHookSlot') + 1), + ); + const mkdirCatch = slotFn.slice( + slotFn.indexOf('fs.mkdirSync(lockDir'), + slotFn.indexOf('const myPidStr'), + ); + expect(mkdirCatch).toContain('return null'); + expect(mkdirCatch).not.toMatch(/return\s*\(\s*\)\s*=>\s*\{\s*\}/); + }); } });