mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-25 01:01:28 +00:00
fix(hooks): cap concurrent augment subprocesses to prevent runaway process spawn (#1486)
When Claude Code fires PreToolUse hooks for parallel Grep/Glob/Bash tool calls, each invocation spawned its own `gitnexus augment` subprocess — a Node + LadybugDB cold start that holds resources for several seconds. Under heavy parallel search load (issue #1486: 180+ piled-up processes, load avg > 100), these accumulated faster than they completed because nothing capped concurrent in-flight augments. Add a lockfile-based concurrency guard under `<.gitnexus>/.hook-locks/`: each running hook claims a `<pid>.lock`, the guard counts live PIDs and prunes stale entries (>30s mtime or pid no longer alive), and bails silently when MAX_INFLIGHT (3) is reached. Augment is best-effort enrichment — missing a few fires under burst load is preferable to melting the system. Applied to all three hook variants that spawn augment: - gitnexus/hooks/claude/gitnexus-hook.cjs (npm-installed Claude hook) - gitnexus-claude-plugin/hooks/gitnexus-hook.js (plugin Claude hook) - gitnexus-cursor-integration/hooks/gitnexus-hook.cjs (Cursor hook) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
fcab1e2e82
commit
ee72183326
4 changed files with 390 additions and 3 deletions
|
|
@ -160,6 +160,88 @@ function extractPattern(toolName, toolInput) {
|
|||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Concurrency guard for the augment subprocess.
|
||||
*
|
||||
* Claude Code fires PreToolUse hooks per parallel tool call. With no cap, N
|
||||
* parallel Grep/Glob/Bash tool calls spawn N concurrent `gitnexus augment`
|
||||
* subprocesses — each a Node + LadybugDB cold start that holds resources
|
||||
* for several seconds. Issue #1486 reported 180+ piled-up processes and
|
||||
* load average > 100. We cap in-flight augments to MAX_INFLIGHT and skip
|
||||
* silently above that. Augment is a best-effort enrichment; missing a few
|
||||
* fires under heavy parallel load is preferable to melting the box.
|
||||
*
|
||||
* Implementation: lockfiles named `<pid>.lock` under `<.gitnexus>/.hook-locks/`.
|
||||
* Stale entries are pruned by age (>30s mtime) or by pid-liveness check.
|
||||
*/
|
||||
const HOOK_LOCK_SUBDIR = '.hook-locks';
|
||||
const HOOK_LOCK_MAX_INFLIGHT = 3;
|
||||
const HOOK_LOCK_STALE_MS = 30000;
|
||||
|
||||
function acquireHookSlot(gitNexusDir) {
|
||||
const lockDir = path.join(gitNexusDir, HOOK_LOCK_SUBDIR);
|
||||
let entries;
|
||||
try {
|
||||
fs.mkdirSync(lockDir, { recursive: true });
|
||||
entries = fs.readdirSync(lockDir);
|
||||
} catch {
|
||||
return () => {};
|
||||
}
|
||||
|
||||
const now = Date.now();
|
||||
let active = 0;
|
||||
for (const entry of entries) {
|
||||
if (!entry.endsWith('.lock')) continue;
|
||||
const lockPath = path.join(lockDir, entry);
|
||||
const pid = Number.parseInt(entry, 10);
|
||||
let stale = false;
|
||||
try {
|
||||
const stat = fs.statSync(lockPath);
|
||||
if (now - stat.mtimeMs > HOOK_LOCK_STALE_MS) stale = true;
|
||||
} catch {
|
||||
stale = true;
|
||||
}
|
||||
if (!stale && Number.isFinite(pid) && pid > 0) {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
active++;
|
||||
continue;
|
||||
} catch {
|
||||
stale = true;
|
||||
}
|
||||
}
|
||||
if (stale) {
|
||||
try {
|
||||
fs.unlinkSync(lockPath);
|
||||
} catch {
|
||||
/* another hook beat us to it */
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (active >= HOOK_LOCK_MAX_INFLIGHT) return null;
|
||||
|
||||
const ownLock = path.join(lockDir, `${process.pid}.lock`);
|
||||
try {
|
||||
fs.writeFileSync(ownLock, '', { flag: 'wx' });
|
||||
} catch {
|
||||
return () => {};
|
||||
}
|
||||
|
||||
let released = false;
|
||||
const release = () => {
|
||||
if (released) return;
|
||||
released = true;
|
||||
try {
|
||||
fs.unlinkSync(ownLock);
|
||||
} catch {
|
||||
/* pruned by another hook */
|
||||
}
|
||||
};
|
||||
process.on('exit', release);
|
||||
return release;
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn a gitnexus CLI command synchronously.
|
||||
* Detects binary on PATH once, then runs exactly once.
|
||||
|
|
@ -217,7 +299,8 @@ function sendHookResponse(hookEventName, message) {
|
|||
function handlePreToolUse(input) {
|
||||
const cwd = input.cwd || process.cwd();
|
||||
if (!path.isAbsolute(cwd)) return;
|
||||
if (!findGitNexusDir(cwd)) return;
|
||||
const gitNexusDir = findGitNexusDir(cwd);
|
||||
if (!gitNexusDir) return;
|
||||
|
||||
const toolName = input.tool_name || '';
|
||||
const toolInput = input.tool_input || {};
|
||||
|
|
@ -227,6 +310,9 @@ function handlePreToolUse(input) {
|
|||
const pattern = extractPattern(toolName, toolInput);
|
||||
if (!pattern || pattern.length < 3) return;
|
||||
|
||||
const release = acquireHookSlot(gitNexusDir);
|
||||
if (!release) return;
|
||||
|
||||
let result = '';
|
||||
try {
|
||||
const child = runGitNexusCli(['augment', '--', pattern], cwd, 7000);
|
||||
|
|
@ -235,6 +321,8 @@ function handlePreToolUse(input) {
|
|||
}
|
||||
} catch {
|
||||
/* graceful failure */
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
|
||||
if (result && result.trim()) {
|
||||
|
|
|
|||
|
|
@ -192,6 +192,86 @@ function resolveCliPath() {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Concurrency guard for the augment subprocess.
|
||||
*
|
||||
* Editors fire postToolUse hooks per parallel tool call. With no cap, N
|
||||
* parallel Grep/Read/Shell tool calls spawn N concurrent `gitnexus augment`
|
||||
* subprocesses — each a Node + LadybugDB cold start that holds resources
|
||||
* for several seconds. Issue #1486 reported 180+ piled-up processes and
|
||||
* load average > 100 on the Claude variant; the same shape applies here.
|
||||
*
|
||||
* Implementation: lockfiles named `<pid>.lock` under `<.gitnexus>/.hook-locks/`.
|
||||
* Stale entries are pruned by age (>30s mtime) or by pid-liveness check.
|
||||
*/
|
||||
const HOOK_LOCK_SUBDIR = '.hook-locks';
|
||||
const HOOK_LOCK_MAX_INFLIGHT = 3;
|
||||
const HOOK_LOCK_STALE_MS = 30000;
|
||||
|
||||
function acquireHookSlot(gitNexusDir) {
|
||||
const lockDir = path.join(gitNexusDir, HOOK_LOCK_SUBDIR);
|
||||
let entries;
|
||||
try {
|
||||
fs.mkdirSync(lockDir, { recursive: true });
|
||||
entries = fs.readdirSync(lockDir);
|
||||
} catch {
|
||||
return () => {};
|
||||
}
|
||||
|
||||
const now = Date.now();
|
||||
let active = 0;
|
||||
for (const entry of entries) {
|
||||
if (!entry.endsWith('.lock')) continue;
|
||||
const lockPath = path.join(lockDir, entry);
|
||||
const pid = Number.parseInt(entry, 10);
|
||||
let stale = false;
|
||||
try {
|
||||
const stat = fs.statSync(lockPath);
|
||||
if (now - stat.mtimeMs > HOOK_LOCK_STALE_MS) stale = true;
|
||||
} catch {
|
||||
stale = true;
|
||||
}
|
||||
if (!stale && Number.isFinite(pid) && pid > 0) {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
active++;
|
||||
continue;
|
||||
} catch {
|
||||
stale = true;
|
||||
}
|
||||
}
|
||||
if (stale) {
|
||||
try {
|
||||
fs.unlinkSync(lockPath);
|
||||
} catch {
|
||||
/* another hook beat us to it */
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (active >= HOOK_LOCK_MAX_INFLIGHT) return null;
|
||||
|
||||
const ownLock = path.join(lockDir, `${process.pid}.lock`);
|
||||
try {
|
||||
fs.writeFileSync(ownLock, '', { flag: 'wx' });
|
||||
} catch {
|
||||
return () => {};
|
||||
}
|
||||
|
||||
let released = false;
|
||||
const release = () => {
|
||||
if (released) return;
|
||||
released = true;
|
||||
try {
|
||||
fs.unlinkSync(ownLock);
|
||||
} catch {
|
||||
/* pruned by another hook */
|
||||
}
|
||||
};
|
||||
process.on('exit', release);
|
||||
return release;
|
||||
}
|
||||
|
||||
function runGitNexusCli(cliPath, args, cwd, timeout) {
|
||||
const isWin = process.platform === 'win32';
|
||||
if (cliPath) {
|
||||
|
|
@ -227,7 +307,8 @@ function main() {
|
|||
}
|
||||
const cwd = input.cwd || process.cwd();
|
||||
if (!path.isAbsolute(cwd)) return;
|
||||
if (!findGitNexusDir(cwd)) return;
|
||||
const gitNexusDir = findGitNexusDir(cwd);
|
||||
if (!gitNexusDir) return;
|
||||
|
||||
const toolName = input.tool_name || '';
|
||||
const toolInput = input.tool_input || {};
|
||||
|
|
@ -235,6 +316,9 @@ function main() {
|
|||
const pattern = extractPattern(toolName, toolInput);
|
||||
if (!pattern || pattern.length < 3) return;
|
||||
|
||||
const release = acquireHookSlot(gitNexusDir);
|
||||
if (!release) return;
|
||||
|
||||
const cliPath = resolveCliPath();
|
||||
let result = '';
|
||||
try {
|
||||
|
|
@ -244,6 +328,8 @@ function main() {
|
|||
}
|
||||
} catch {
|
||||
/* graceful failure */
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
|
||||
if (result && result.trim()) {
|
||||
|
|
|
|||
|
|
@ -160,6 +160,90 @@ function extractPattern(toolName, toolInput) {
|
|||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Concurrency guard for the augment subprocess.
|
||||
*
|
||||
* Claude Code fires PreToolUse hooks per parallel tool call. With no cap, N
|
||||
* parallel Grep/Glob/Bash tool calls spawn N concurrent `gitnexus augment`
|
||||
* subprocesses — each a Node + LadybugDB cold start that holds resources
|
||||
* for several seconds. Issue #1486 reported 180+ piled-up processes and
|
||||
* load average > 100. We cap in-flight augments to MAX_INFLIGHT and skip
|
||||
* silently above that. Augment is a best-effort enrichment; missing a few
|
||||
* fires under heavy parallel load is preferable to melting the box.
|
||||
*
|
||||
* Implementation: lockfiles named `<pid>.lock` under `<.gitnexus>/.hook-locks/`.
|
||||
* Stale entries are pruned by age (>30s mtime) or by pid-liveness check.
|
||||
*/
|
||||
const HOOK_LOCK_SUBDIR = '.hook-locks';
|
||||
const HOOK_LOCK_MAX_INFLIGHT = 3;
|
||||
const HOOK_LOCK_STALE_MS = 30000;
|
||||
|
||||
function acquireHookSlot(gitNexusDir) {
|
||||
const lockDir = path.join(gitNexusDir, HOOK_LOCK_SUBDIR);
|
||||
let entries;
|
||||
try {
|
||||
fs.mkdirSync(lockDir, { recursive: true });
|
||||
entries = fs.readdirSync(lockDir);
|
||||
} catch {
|
||||
// Cannot read lock dir — fall through unguarded so the hook still works.
|
||||
return () => {};
|
||||
}
|
||||
|
||||
const now = Date.now();
|
||||
let active = 0;
|
||||
for (const entry of entries) {
|
||||
if (!entry.endsWith('.lock')) continue;
|
||||
const lockPath = path.join(lockDir, entry);
|
||||
const pid = Number.parseInt(entry, 10);
|
||||
let stale = false;
|
||||
try {
|
||||
const stat = fs.statSync(lockPath);
|
||||
if (now - stat.mtimeMs > HOOK_LOCK_STALE_MS) stale = true;
|
||||
} catch {
|
||||
stale = true;
|
||||
}
|
||||
if (!stale && Number.isFinite(pid) && pid > 0) {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
active++;
|
||||
continue;
|
||||
} catch {
|
||||
stale = true;
|
||||
}
|
||||
}
|
||||
if (stale) {
|
||||
try {
|
||||
fs.unlinkSync(lockPath);
|
||||
} catch {
|
||||
/* another hook beat us to it */
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (active >= HOOK_LOCK_MAX_INFLIGHT) return null;
|
||||
|
||||
const ownLock = path.join(lockDir, `${process.pid}.lock`);
|
||||
try {
|
||||
fs.writeFileSync(ownLock, '', { flag: 'wx' });
|
||||
} catch {
|
||||
// Couldn't claim a slot — proceed unguarded rather than block legitimate work.
|
||||
return () => {};
|
||||
}
|
||||
|
||||
let released = false;
|
||||
const release = () => {
|
||||
if (released) return;
|
||||
released = true;
|
||||
try {
|
||||
fs.unlinkSync(ownLock);
|
||||
} catch {
|
||||
/* pruned by another hook */
|
||||
}
|
||||
};
|
||||
process.on('exit', release);
|
||||
return release;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the gitnexus CLI path.
|
||||
* 1. Relative path (works when script is inside npm package)
|
||||
|
|
@ -207,7 +291,8 @@ function runGitNexusCli(cliPath, args, cwd, timeout) {
|
|||
function handlePreToolUse(input) {
|
||||
const cwd = input.cwd || process.cwd();
|
||||
if (!path.isAbsolute(cwd)) return;
|
||||
if (!findGitNexusDir(cwd)) return;
|
||||
const gitNexusDir = findGitNexusDir(cwd);
|
||||
if (!gitNexusDir) return;
|
||||
|
||||
const toolName = input.tool_name || '';
|
||||
const toolInput = input.tool_input || {};
|
||||
|
|
@ -217,6 +302,9 @@ function handlePreToolUse(input) {
|
|||
const pattern = extractPattern(toolName, toolInput);
|
||||
if (!pattern || pattern.length < 3) return;
|
||||
|
||||
const release = acquireHookSlot(gitNexusDir);
|
||||
if (!release) return;
|
||||
|
||||
const cliPath = resolveCliPath();
|
||||
let result = '';
|
||||
try {
|
||||
|
|
@ -226,6 +314,8 @@ function handlePreToolUse(input) {
|
|||
}
|
||||
} catch {
|
||||
/* graceful failure */
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
|
||||
if (result && result.trim()) {
|
||||
|
|
|
|||
|
|
@ -294,6 +294,129 @@ describe('Git mutation regex', () => {
|
|||
}
|
||||
});
|
||||
|
||||
// ─── Source code regression: PreToolUse concurrency guard (#1486) ──
|
||||
|
||||
describe('PreToolUse concurrency guard', () => {
|
||||
for (const [label, hookPath] of [
|
||||
['CJS', CJS_HOOK],
|
||||
['Plugin', PLUGIN_HOOK],
|
||||
] as const) {
|
||||
it(`${label} hook defines acquireHookSlot`, () => {
|
||||
const source = fs.readFileSync(hookPath, 'utf-8');
|
||||
expect(source).toContain('function acquireHookSlot');
|
||||
expect(source).toContain('HOOK_LOCK_MAX_INFLIGHT');
|
||||
});
|
||||
|
||||
it(`${label} hook calls acquireHookSlot in handlePreToolUse`, () => {
|
||||
const source = fs.readFileSync(hookPath, 'utf-8');
|
||||
const preBody = source.slice(
|
||||
source.indexOf('function handlePreToolUse'),
|
||||
source.indexOf('function handlePostToolUse'),
|
||||
);
|
||||
expect(preBody).toContain('acquireHookSlot(');
|
||||
expect(preBody).toMatch(/release\(\)/);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// ─── Integration: concurrency guard skips when slots are full ──────
|
||||
|
||||
describe('PreToolUse concurrency guard (integration)', () => {
|
||||
for (const [label, hookPath] of [
|
||||
['CJS', CJS_HOOK],
|
||||
['Plugin', PLUGIN_HOOK],
|
||||
] as const) {
|
||||
it(`${label}: hook exits silently when MAX_INFLIGHT lockfiles for live pids exist`, async () => {
|
||||
const { spawn } = await import('child_process');
|
||||
const lockDir = path.join(gitNexusDir, '.hook-locks');
|
||||
fs.mkdirSync(lockDir, { recursive: true });
|
||||
|
||||
// Spawn 3 long-sleeping node child processes to use as live PIDs.
|
||||
const sleepers = [0, 1, 2].map(() =>
|
||||
spawn(process.execPath, ['-e', 'setTimeout(()=>{},60000)'], {
|
||||
stdio: 'ignore',
|
||||
detached: false,
|
||||
}),
|
||||
);
|
||||
const writtenLocks: string[] = [];
|
||||
try {
|
||||
for (const child of sleepers) {
|
||||
const p = path.join(lockDir, `${child.pid}.lock`);
|
||||
fs.writeFileSync(p, '');
|
||||
writtenLocks.push(p);
|
||||
}
|
||||
|
||||
const result = runHook(hookPath, {
|
||||
hook_event_name: 'PreToolUse',
|
||||
tool_name: 'Grep',
|
||||
tool_input: { pattern: 'validateUser' },
|
||||
cwd: tmpDir,
|
||||
});
|
||||
|
||||
expect(result.stdout.trim()).toBe('');
|
||||
const afterFiles = fs.readdirSync(lockDir).filter((f) => f.endsWith('.lock'));
|
||||
// Sentinel locks remain; the hook bailed before acquiring its own slot.
|
||||
expect(afterFiles.length).toBe(3);
|
||||
} finally {
|
||||
for (const child of sleepers) {
|
||||
try {
|
||||
child.kill();
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
for (const p of writtenLocks) {
|
||||
try {
|
||||
fs.unlinkSync(p);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
try {
|
||||
fs.rmdirSync(lockDir);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it(`${label}: hook prunes stale lockfiles (dead pid)`, () => {
|
||||
const lockDir = path.join(gitNexusDir, '.hook-locks');
|
||||
fs.mkdirSync(lockDir, { recursive: true });
|
||||
// PID 1 exists on every POSIX system (init); on Windows process.kill(1,0)
|
||||
// throws. Use a definitely-dead PID instead: a very large number unlikely
|
||||
// to be assigned.
|
||||
const deadPid = 2_147_483_640;
|
||||
const stalePath = path.join(lockDir, `${deadPid}.lock`);
|
||||
try {
|
||||
fs.writeFileSync(stalePath, '');
|
||||
expect(fs.existsSync(stalePath)).toBe(true);
|
||||
|
||||
runHook(hookPath, {
|
||||
hook_event_name: 'PreToolUse',
|
||||
tool_name: 'Grep',
|
||||
tool_input: { pattern: 'validateUser' },
|
||||
cwd: tmpDir,
|
||||
});
|
||||
|
||||
// After the hook runs, the dead-pid lockfile should be pruned.
|
||||
expect(fs.existsSync(stalePath)).toBe(false);
|
||||
} finally {
|
||||
try {
|
||||
fs.unlinkSync(stalePath);
|
||||
} catch {
|
||||
/* already pruned */
|
||||
}
|
||||
try {
|
||||
fs.rmdirSync(lockDir);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// ─── Integration: PostToolUse staleness detection ───────────────────
|
||||
|
||||
describe('PostToolUse staleness detection (integration)', () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue