diff --git a/gitnexus-claude-plugin/hooks/gitnexus-hook.js b/gitnexus-claude-plugin/hooks/gitnexus-hook.js index 35945df04..245d34043 100644 --- a/gitnexus-claude-plugin/hooks/gitnexus-hook.js +++ b/gitnexus-claude-plugin/hooks/gitnexus-hook.js @@ -14,6 +14,7 @@ const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); +const { acquireHookSlot } = require('./hook-lock.js'); /** * Read JSON input from stdin synchronously. @@ -160,140 +161,6 @@ function extractPattern(toolName, toolInput) { return null; } -/** - * Concurrency guard for the augment subprocess (hard cap). - * - * 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: fixed-name slot files `slot-0.lock` ... `slot-N.lock` - * under `<.gitnexus>/.hook-locks/`. Each file is created with `wx` - * (O_CREAT|O_EXCL), which is atomic across processes at the OS level — - * exactly one process wins each slot. This is a HARD cap, not the - * count-then-claim soft cap that an earlier revision shipped (it had a - * TOCTOU window between readdirSync and the per-pid wx write). - * - * Owner identity is written into the slot file as the PID, used for - * stale-takeover when a hook crashes without releasing. PID-liveness is - * checked before age so a slow-but-alive hook is never wrongly evicted; - * the age window only kicks in to defend against PID reuse on a long- - * abandoned slot. - */ -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); - try { - fs.mkdirSync(lockDir, { recursive: true }); - } catch { - // 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); - - for (let slot = 0; slot < HOOK_LOCK_MAX_INFLIGHT; slot++) { - const slotPath = path.join(lockDir, `slot-${slot}.lock`); - for (let attempt = 0; attempt < 2; attempt++) { - try { - fs.writeFileSync(slotPath, myPidStr, { flag: 'wx' }); - let released = false; - const release = () => { - if (released) return; - released = true; - try { - // Only unlink if we still own the slot. If we appeared stale and - // another hook took over, the file now belongs to it — leave alone. - const content = fs.readFileSync(slotPath, 'utf-8').trim(); - if (content === myPidStr) fs.unlinkSync(slotPath); - } catch { - /* already removed or unreadable */ - } - }; - process.on('exit', release); - return release; - } catch { - // Slot exists. Decide whether to take it over. - // Open once and inspect mtime + content via the same fd so there's - // no TOCTOU between the metadata check and the content read - // (codeql js/file-system-race). - let fd; - try { - fd = fs.openSync(slotPath, 'r'); - } catch { - continue; // Vanished between EEXIST and open — retry this slot. - } - let isLive = false; - let mtimeMs = Date.now(); - try { - mtimeMs = fs.fstatSync(fd).mtimeMs; - const buf = Buffer.alloc(32); - const n = fs.readSync(fd, buf, 0, 32, 0); - const ownerStr = buf.slice(0, n).toString('utf-8').trim(); - if (ownerStr === '') { - // Owner created the file but hasn't written its PID yet. The - // wx open+write window is microseconds; give it the benefit - // of the doubt and treat as live. - isLive = true; - } else { - const owner = Number.parseInt(ownerStr, 10); - if (Number.isFinite(owner) && owner > 0) { - try { - process.kill(owner, 0); - isLive = true; - } catch (e) { - // ESRCH = process gone → treat as dead. EPERM = process exists - // but owned by another user (cross-user lock dir) → still alive, - // keep the slot. Anything else: be conservative, assume alive. - if (e && e.code === 'ESRCH') { - isLive = false; - } else { - isLive = true; - } - } - } - } - } catch { - /* unreadable — treat as dead */ - } finally { - try { - fs.closeSync(fd); - } catch { - /* already closed */ - } - } - // 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; - } - if (isLive) break; // Try the next slot. - try { - fs.unlinkSync(slotPath); - } catch { - /* another hook beat us to it — retry will hit EEXIST */ - } - // Loop and retry this slot. - } - } - } - - return null; -} - /** * Spawn a gitnexus CLI command synchronously. * Detects binary on PATH once, then runs exactly once. diff --git a/gitnexus-claude-plugin/hooks/hook-lock.js b/gitnexus-claude-plugin/hooks/hook-lock.js new file mode 100644 index 000000000..759856384 --- /dev/null +++ b/gitnexus-claude-plugin/hooks/hook-lock.js @@ -0,0 +1,119 @@ +const fs = require('fs'); +const path = require('path'); + +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); + try { + fs.mkdirSync(lockDir, { recursive: true }); + } catch { + // 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); + + for (let slot = 0; slot < HOOK_LOCK_MAX_INFLIGHT; slot++) { + const slotPath = path.join(lockDir, `slot-${slot}.lock`); + for (let attempt = 0; attempt < 2; attempt++) { + try { + fs.writeFileSync(slotPath, myPidStr, { flag: 'wx' }); + let released = false; + const release = () => { + if (released) return; + released = true; + try { + // Only unlink if we still own the slot. If we appeared stale and + // another hook took over, the file now belongs to it — leave alone. + const content = fs.readFileSync(slotPath, 'utf-8').trim(); + if (content === myPidStr) fs.unlinkSync(slotPath); + } catch { + /* already removed or unreadable */ + } + }; + process.on('exit', release); + return release; + } catch { + // Slot exists. Decide whether to take it over. + // Open once and inspect mtime + content via the same fd so there's + // no TOCTOU between the metadata check and the content read + // (codeql js/file-system-race). + let fd; + try { + fd = fs.openSync(slotPath, 'r'); + } catch { + continue; // Vanished between EEXIST and open — retry this slot. + } + let isLive = false; + let mtimeMs = Date.now(); + try { + mtimeMs = fs.fstatSync(fd).mtimeMs; + const buf = Buffer.alloc(32); + const n = fs.readSync(fd, buf, 0, 32, 0); + const ownerStr = buf.slice(0, n).toString('utf-8').trim(); + if (ownerStr === '') { + // Owner created the file but hasn't written its PID yet. The + // wx open+write window is microseconds; give it the benefit + // of the doubt and treat as live. + isLive = true; + } else { + const owner = Number.parseInt(ownerStr, 10); + if (Number.isFinite(owner) && owner > 0) { + try { + process.kill(owner, 0); + isLive = true; + } catch (e) { + // ESRCH = process gone → treat as dead. EPERM = process exists + // but owned by another user (cross-user lock dir) → still alive, + // keep the slot. Anything else: be conservative, assume alive. + if (e && e.code === 'ESRCH') { + isLive = false; + } else { + isLive = true; + } + } + } + } + } catch { + /* unreadable — treat as dead */ + } finally { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + } + // 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; + } + if (isLive) break; // Try the next slot. + try { + fs.unlinkSync(slotPath); + } catch { + /* another hook beat us to it — retry will hit EEXIST */ + } + // Loop and retry this slot. + } + } + } + + return null; +} + +module.exports = { + HOOK_LOCK_SUBDIR, + HOOK_LOCK_MAX_INFLIGHT, + HOOK_LOCK_STALE_MS, + acquireHookSlot, +}; diff --git a/gitnexus-cursor-integration/README.md b/gitnexus-cursor-integration/README.md index 0da8b1981..6545bacec 100644 --- a/gitnexus-cursor-integration/README.md +++ b/gitnexus-cursor-integration/README.md @@ -10,20 +10,21 @@ Static config that adds GitNexus knowledge-graph augmentation and skill files to | ------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------- | | **MCP** | `gitnexus` MCP server with 16 tools (`query`, `context`, `impact`, `detect_changes`, `rename`, …) | `npx gitnexus setup` writes `~/.cursor/mcp.json` automatically. | | **Skills** | `/gitnexus-exploring`, `/gitnexus-debugging`, `/gitnexus-impact-analysis`, `/gitnexus-refactoring`, `/gitnexus-pr-review` markdown skills | `npx gitnexus setup` copies them to `~/.cursor/skills/gitnexus/`. | -| **Hooks** _(this README)_ | `postToolUse` hook that enriches `Shell` / `Read` / `Grep` tool calls with graph context — same augmentation Claude Code gets | **Manual** — copy the two files described below into your project's `.cursor/`. | +| **Hooks** _(this README)_ | `postToolUse` hook that enriches `Shell` / `Read` / `Grep` tool calls with graph context — same augmentation Claude Code gets | **Manual** — copy the files described below into your project's `.cursor/`. | ## Hook install Cursor 2.4+ reads `.cursor/hooks.json` from the project root and runs hook commands with the project root as the working directory ([docs](https://cursor.com/docs/agent/hooks)). -From this repo's `gitnexus-cursor-integration/hooks/`, copy the two files into your **project root**: +From this repo's `gitnexus-cursor-integration/hooks/`, copy the files below into your **project root**: ```text / ├── .cursor/ │ └── hooks.json ← from gitnexus-cursor-integration/hooks/hooks.json └── hooks/ - └── gitnexus-hook.cjs ← from gitnexus-cursor-integration/hooks/gitnexus-hook.cjs + ├── gitnexus-hook.cjs ← from gitnexus-cursor-integration/hooks/gitnexus-hook.cjs + └── hook-lock.cjs ← from gitnexus-cursor-integration/hooks/hook-lock.cjs ``` Equivalent shell commands (run from your project root, with `$GITNEXUS_REPO` pointing at a clone of this repo): @@ -32,6 +33,7 @@ Equivalent shell commands (run from your project root, with `$GITNEXUS_REPO` poi mkdir -p .cursor hooks cp "$GITNEXUS_REPO/gitnexus-cursor-integration/hooks/hooks.json" .cursor/hooks.json cp "$GITNEXUS_REPO/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs" hooks/gitnexus-hook.cjs +cp "$GITNEXUS_REPO/gitnexus-cursor-integration/hooks/hook-lock.cjs" hooks/hook-lock.cjs ``` If you already have a `.cursor/hooks.json`, merge the `hooks.postToolUse` array rather than overwriting. @@ -49,7 +51,7 @@ If you already have a `.cursor/hooks.json`, merge the `hooks.postToolUse` array | -------------------------------------------------------------------- | ------------------------------ | | `~/.cursor/mcp.json` | ✅ | | `~/.cursor/skills/gitnexus/*` | ✅ | -| `/.cursor/hooks.json` + `/hooks/gitnexus-hook.cjs` | ❌ — copy manually (see above) | +| `/.cursor/hooks.json` + `/hooks/gitnexus-hook.cjs` + `/hooks/hook-lock.cjs` | ❌ — copy manually (see above) | Hook install is per-project (Cursor scopes hooks to a project root); skills and MCP config are global. @@ -84,6 +86,6 @@ Empty stdout means "no augmentation, continue normally" — the hook never block ## Troubleshooting -- **Nothing happens** — Confirm Cursor is on 2.4+ and the project root has both `.cursor/hooks.json` and the script at `hooks/gitnexus-hook.cjs`. Then `npx gitnexus list` to confirm the project is indexed. +- **Nothing happens** — Confirm Cursor is on 2.4+ and the project root has `.cursor/hooks.json` plus both hook files at `hooks/gitnexus-hook.cjs` and `hooks/hook-lock.cjs`. Then `npx gitnexus list` to confirm the project is indexed. - **`gitnexus` not found** — The hook prefers a locally-resolvable `gitnexus/dist/cli/index.js` and falls back to `npx -y gitnexus`. Install globally with `npm i -g gitnexus` to skip the npx cold-start latency. - **Wrong pattern extracted** — Set `GITNEXUS_DEBUG=1` and run a tool call. The raw stdin payload is logged to stderr; use it to confirm Cursor's actual `tool_input` field names against the table above. If they differ, file an issue with the captured payload. diff --git a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs index 010e86157..74c5587b3 100644 --- a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs +++ b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs @@ -18,6 +18,7 @@ const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); +const { acquireHookSlot } = require('./hook-lock.cjs'); function readInput() { try { @@ -192,138 +193,6 @@ function resolveCliPath() { } } -/** - * Concurrency guard for the augment subprocess (hard cap). - * - * 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: fixed-name slot files `slot-0.lock` ... `slot-N.lock` - * under `<.gitnexus>/.hook-locks/`. Each file is created with `wx` - * (O_CREAT|O_EXCL), which is atomic across processes at the OS level — - * exactly one process wins each slot. This is a HARD cap, not the - * count-then-claim soft cap that an earlier revision shipped (it had a - * TOCTOU window between readdirSync and the per-pid wx write). - * - * Owner identity is written into the slot file as the PID, used for - * stale-takeover when a hook crashes without releasing. PID-liveness is - * checked before age so a slow-but-alive hook is never wrongly evicted; - * the age window only kicks in to defend against PID reuse on a long- - * abandoned slot. - */ -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); - try { - fs.mkdirSync(lockDir, { recursive: true }); - } catch { - // 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); - - for (let slot = 0; slot < HOOK_LOCK_MAX_INFLIGHT; slot++) { - const slotPath = path.join(lockDir, `slot-${slot}.lock`); - for (let attempt = 0; attempt < 2; attempt++) { - try { - fs.writeFileSync(slotPath, myPidStr, { flag: 'wx' }); - let released = false; - const release = () => { - if (released) return; - released = true; - try { - // Only unlink if we still own the slot. If we appeared stale and - // another hook took over, the file now belongs to it — leave alone. - const content = fs.readFileSync(slotPath, 'utf-8').trim(); - if (content === myPidStr) fs.unlinkSync(slotPath); - } catch { - /* already removed or unreadable */ - } - }; - process.on('exit', release); - return release; - } catch { - // Slot exists. Decide whether to take it over. - // Open once and inspect mtime + content via the same fd so there's - // no TOCTOU between the metadata check and the content read - // (codeql js/file-system-race). - let fd; - try { - fd = fs.openSync(slotPath, 'r'); - } catch { - continue; // Vanished between EEXIST and open — retry this slot. - } - let isLive = false; - let mtimeMs = Date.now(); - try { - mtimeMs = fs.fstatSync(fd).mtimeMs; - const buf = Buffer.alloc(32); - const n = fs.readSync(fd, buf, 0, 32, 0); - const ownerStr = buf.slice(0, n).toString('utf-8').trim(); - if (ownerStr === '') { - // Owner created the file but hasn't written its PID yet. The - // wx open+write window is microseconds; give it the benefit - // of the doubt and treat as live. - isLive = true; - } else { - const owner = Number.parseInt(ownerStr, 10); - if (Number.isFinite(owner) && owner > 0) { - try { - process.kill(owner, 0); - isLive = true; - } catch (e) { - // ESRCH = process gone → treat as dead. EPERM = process exists - // but owned by another user (cross-user lock dir) → still alive, - // keep the slot. Anything else: be conservative, assume alive. - if (e && e.code === 'ESRCH') { - isLive = false; - } else { - isLive = true; - } - } - } - } - } catch { - /* unreadable — treat as dead */ - } finally { - try { - fs.closeSync(fd); - } catch { - /* already closed */ - } - } - // 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; - } - if (isLive) break; // Try the next slot. - try { - fs.unlinkSync(slotPath); - } catch { - /* another hook beat us to it — retry will hit EEXIST */ - } - // Loop and retry this slot. - } - } - } - - return null; -} - function runGitNexusCli(cliPath, args, cwd, timeout) { const isWin = process.platform === 'win32'; if (cliPath) { diff --git a/gitnexus-cursor-integration/hooks/hook-lock.cjs b/gitnexus-cursor-integration/hooks/hook-lock.cjs new file mode 100644 index 000000000..759856384 --- /dev/null +++ b/gitnexus-cursor-integration/hooks/hook-lock.cjs @@ -0,0 +1,119 @@ +const fs = require('fs'); +const path = require('path'); + +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); + try { + fs.mkdirSync(lockDir, { recursive: true }); + } catch { + // 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); + + for (let slot = 0; slot < HOOK_LOCK_MAX_INFLIGHT; slot++) { + const slotPath = path.join(lockDir, `slot-${slot}.lock`); + for (let attempt = 0; attempt < 2; attempt++) { + try { + fs.writeFileSync(slotPath, myPidStr, { flag: 'wx' }); + let released = false; + const release = () => { + if (released) return; + released = true; + try { + // Only unlink if we still own the slot. If we appeared stale and + // another hook took over, the file now belongs to it — leave alone. + const content = fs.readFileSync(slotPath, 'utf-8').trim(); + if (content === myPidStr) fs.unlinkSync(slotPath); + } catch { + /* already removed or unreadable */ + } + }; + process.on('exit', release); + return release; + } catch { + // Slot exists. Decide whether to take it over. + // Open once and inspect mtime + content via the same fd so there's + // no TOCTOU between the metadata check and the content read + // (codeql js/file-system-race). + let fd; + try { + fd = fs.openSync(slotPath, 'r'); + } catch { + continue; // Vanished between EEXIST and open — retry this slot. + } + let isLive = false; + let mtimeMs = Date.now(); + try { + mtimeMs = fs.fstatSync(fd).mtimeMs; + const buf = Buffer.alloc(32); + const n = fs.readSync(fd, buf, 0, 32, 0); + const ownerStr = buf.slice(0, n).toString('utf-8').trim(); + if (ownerStr === '') { + // Owner created the file but hasn't written its PID yet. The + // wx open+write window is microseconds; give it the benefit + // of the doubt and treat as live. + isLive = true; + } else { + const owner = Number.parseInt(ownerStr, 10); + if (Number.isFinite(owner) && owner > 0) { + try { + process.kill(owner, 0); + isLive = true; + } catch (e) { + // ESRCH = process gone → treat as dead. EPERM = process exists + // but owned by another user (cross-user lock dir) → still alive, + // keep the slot. Anything else: be conservative, assume alive. + if (e && e.code === 'ESRCH') { + isLive = false; + } else { + isLive = true; + } + } + } + } + } catch { + /* unreadable — treat as dead */ + } finally { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + } + // 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; + } + if (isLive) break; // Try the next slot. + try { + fs.unlinkSync(slotPath); + } catch { + /* another hook beat us to it — retry will hit EEXIST */ + } + // Loop and retry this slot. + } + } + } + + return null; +} + +module.exports = { + HOOK_LOCK_SUBDIR, + HOOK_LOCK_MAX_INFLIGHT, + HOOK_LOCK_STALE_MS, + acquireHookSlot, +}; diff --git a/gitnexus/hooks/claude/gitnexus-hook.cjs b/gitnexus/hooks/claude/gitnexus-hook.cjs index 446b758af..9541fcb50 100755 --- a/gitnexus/hooks/claude/gitnexus-hook.cjs +++ b/gitnexus/hooks/claude/gitnexus-hook.cjs @@ -14,6 +14,7 @@ const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); +const { acquireHookSlot } = require('./hook-lock.cjs'); /** * Read JSON input from stdin synchronously. @@ -160,140 +161,6 @@ function extractPattern(toolName, toolInput) { return null; } -/** - * Concurrency guard for the augment subprocess (hard cap). - * - * 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: fixed-name slot files `slot-0.lock` ... `slot-N.lock` - * under `<.gitnexus>/.hook-locks/`. Each file is created with `wx` - * (O_CREAT|O_EXCL), which is atomic across processes at the OS level — - * exactly one process wins each slot. This is a HARD cap, not the - * count-then-claim soft cap that an earlier revision shipped (it had a - * TOCTOU window between readdirSync and the per-pid wx write). - * - * Owner identity is written into the slot file as the PID, used for - * stale-takeover when a hook crashes without releasing. PID-liveness is - * checked before age so a slow-but-alive hook is never wrongly evicted; - * the age window only kicks in to defend against PID reuse on a long- - * abandoned slot. - */ -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); - try { - fs.mkdirSync(lockDir, { recursive: true }); - } catch { - // 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); - - for (let slot = 0; slot < HOOK_LOCK_MAX_INFLIGHT; slot++) { - const slotPath = path.join(lockDir, `slot-${slot}.lock`); - for (let attempt = 0; attempt < 2; attempt++) { - try { - fs.writeFileSync(slotPath, myPidStr, { flag: 'wx' }); - let released = false; - const release = () => { - if (released) return; - released = true; - try { - // Only unlink if we still own the slot. If we appeared stale and - // another hook took over, the file now belongs to it — leave alone. - const content = fs.readFileSync(slotPath, 'utf-8').trim(); - if (content === myPidStr) fs.unlinkSync(slotPath); - } catch { - /* already removed or unreadable */ - } - }; - process.on('exit', release); - return release; - } catch { - // Slot exists. Decide whether to take it over. - // Open once and inspect mtime + content via the same fd so there's - // no TOCTOU between the metadata check and the content read - // (codeql js/file-system-race). - let fd; - try { - fd = fs.openSync(slotPath, 'r'); - } catch { - continue; // Vanished between EEXIST and open — retry this slot. - } - let isLive = false; - let mtimeMs = Date.now(); - try { - mtimeMs = fs.fstatSync(fd).mtimeMs; - const buf = Buffer.alloc(32); - const n = fs.readSync(fd, buf, 0, 32, 0); - const ownerStr = buf.slice(0, n).toString('utf-8').trim(); - if (ownerStr === '') { - // Owner created the file but hasn't written its PID yet. The - // wx open+write window is microseconds; give it the benefit - // of the doubt and treat as live. - isLive = true; - } else { - const owner = Number.parseInt(ownerStr, 10); - if (Number.isFinite(owner) && owner > 0) { - try { - process.kill(owner, 0); - isLive = true; - } catch (e) { - // ESRCH = process gone → treat as dead. EPERM = process exists - // but owned by another user (cross-user lock dir) → still alive, - // keep the slot. Anything else: be conservative, assume alive. - if (e && e.code === 'ESRCH') { - isLive = false; - } else { - isLive = true; - } - } - } - } - } catch { - /* unreadable — treat as dead */ - } finally { - try { - fs.closeSync(fd); - } catch { - /* already closed */ - } - } - // 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; - } - if (isLive) break; // Try the next slot. - try { - fs.unlinkSync(slotPath); - } catch { - /* another hook beat us to it — retry will hit EEXIST */ - } - // Loop and retry this slot. - } - } - } - - return null; -} - /** * Resolve the gitnexus CLI path. * 1. Relative path (works when script is inside npm package) diff --git a/gitnexus/hooks/claude/hook-lock.cjs b/gitnexus/hooks/claude/hook-lock.cjs new file mode 100644 index 000000000..759856384 --- /dev/null +++ b/gitnexus/hooks/claude/hook-lock.cjs @@ -0,0 +1,119 @@ +const fs = require('fs'); +const path = require('path'); + +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); + try { + fs.mkdirSync(lockDir, { recursive: true }); + } catch { + // 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); + + for (let slot = 0; slot < HOOK_LOCK_MAX_INFLIGHT; slot++) { + const slotPath = path.join(lockDir, `slot-${slot}.lock`); + for (let attempt = 0; attempt < 2; attempt++) { + try { + fs.writeFileSync(slotPath, myPidStr, { flag: 'wx' }); + let released = false; + const release = () => { + if (released) return; + released = true; + try { + // Only unlink if we still own the slot. If we appeared stale and + // another hook took over, the file now belongs to it — leave alone. + const content = fs.readFileSync(slotPath, 'utf-8').trim(); + if (content === myPidStr) fs.unlinkSync(slotPath); + } catch { + /* already removed or unreadable */ + } + }; + process.on('exit', release); + return release; + } catch { + // Slot exists. Decide whether to take it over. + // Open once and inspect mtime + content via the same fd so there's + // no TOCTOU between the metadata check and the content read + // (codeql js/file-system-race). + let fd; + try { + fd = fs.openSync(slotPath, 'r'); + } catch { + continue; // Vanished between EEXIST and open — retry this slot. + } + let isLive = false; + let mtimeMs = Date.now(); + try { + mtimeMs = fs.fstatSync(fd).mtimeMs; + const buf = Buffer.alloc(32); + const n = fs.readSync(fd, buf, 0, 32, 0); + const ownerStr = buf.slice(0, n).toString('utf-8').trim(); + if (ownerStr === '') { + // Owner created the file but hasn't written its PID yet. The + // wx open+write window is microseconds; give it the benefit + // of the doubt and treat as live. + isLive = true; + } else { + const owner = Number.parseInt(ownerStr, 10); + if (Number.isFinite(owner) && owner > 0) { + try { + process.kill(owner, 0); + isLive = true; + } catch (e) { + // ESRCH = process gone → treat as dead. EPERM = process exists + // but owned by another user (cross-user lock dir) → still alive, + // keep the slot. Anything else: be conservative, assume alive. + if (e && e.code === 'ESRCH') { + isLive = false; + } else { + isLive = true; + } + } + } + } + } catch { + /* unreadable — treat as dead */ + } finally { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + } + // 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; + } + if (isLive) break; // Try the next slot. + try { + fs.unlinkSync(slotPath); + } catch { + /* another hook beat us to it — retry will hit EEXIST */ + } + // Loop and retry this slot. + } + } + } + + return null; +} + +module.exports = { + HOOK_LOCK_SUBDIR, + HOOK_LOCK_MAX_INFLIGHT, + HOOK_LOCK_STALE_MS, + acquireHookSlot, +}; diff --git a/gitnexus/src/cli/setup.ts b/gitnexus/src/cli/setup.ts index af3c4737a..8f52e0f2d 100644 --- a/gitnexus/src/cli/setup.ts +++ b/gitnexus/src/cli/setup.ts @@ -364,6 +364,15 @@ async function installClaudeCodeHooks(result: SetupResult): Promise { // Script not found in source — skip } + try { + await fs.copyFile( + path.join(pluginHooksPath, 'hook-lock.cjs'), + path.join(destHooksDir, 'hook-lock.cjs'), + ); + } catch { + // Helper not found in source — skip + } + const hookPath = path.join(destHooksDir, 'gitnexus-hook.cjs').replace(/\\/g, '/'); // Escape backslashes FIRST, then quotes (CodeQL js/incomplete-sanitization). // The previous shape `replace(/"/g, '\\"')` alone would let `path\with"quote` diff --git a/gitnexus/test/unit/cursor-hook.test.ts b/gitnexus/test/unit/cursor-hook.test.ts index 9d484adf1..e64979895 100644 --- a/gitnexus/test/unit/cursor-hook.test.ts +++ b/gitnexus/test/unit/cursor-hook.test.ts @@ -35,6 +35,15 @@ const CURSOR_HOOK = path.resolve( 'hooks', 'gitnexus-hook.cjs', ); +const CURSOR_HOOK_LOCK = path.resolve( + __dirname, + '..', + '..', + '..', + 'gitnexus-cursor-integration', + 'hooks', + 'hook-lock.cjs', +); const CURSOR_HOOKS_JSON = path.resolve( __dirname, '..', @@ -398,10 +407,16 @@ describe('Cursor hook debug logging', () => { describe('Cursor hook concurrency guard', () => { const source = fs.readFileSync(CURSOR_HOOK, 'utf-8'); + const lockSource = fs.readFileSync(CURSOR_HOOK_LOCK, 'utf-8'); - it('defines acquireHookSlot with MAX_INFLIGHT constant', () => { - expect(source).toContain('function acquireHookSlot'); - expect(source).toContain('HOOK_LOCK_MAX_INFLIGHT'); + it('loads acquireHookSlot helper module', () => { + expect(source).toContain('acquireHookSlot'); + expect(source).toContain('hook-lock.cjs'); + }); + + it('helper defines acquireHookSlot with MAX_INFLIGHT constant', () => { + expect(lockSource).toContain('function acquireHookSlot'); + expect(lockSource).toContain('HOOK_LOCK_MAX_INFLIGHT'); }); it('calls acquireHookSlot in main() and releases via finally', () => { @@ -413,10 +428,10 @@ describe('Cursor hook concurrency guard', () => { }); it('uses atomic fixed-name slot files (hard cap, not soft TOCTOU cap)', () => { - expect(source).toMatch(/slot-\$\{slot\}\.lock|`slot-/); - const slotFn = source.slice( - source.indexOf('function acquireHookSlot'), - source.indexOf('function', source.indexOf('function acquireHookSlot') + 1), + expect(lockSource).toMatch(/slot-\$\{slot\}\.lock|`slot-/); + const slotFn = lockSource.slice( + lockSource.indexOf('function acquireHookSlot'), + lockSource.indexOf('function', lockSource.indexOf('function acquireHookSlot') + 1), ); expect(slotFn).not.toContain('readdirSync'); }); @@ -425,9 +440,9 @@ describe('Cursor hook concurrency guard', () => { // 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 slotFn = lockSource.slice( + lockSource.indexOf('function acquireHookSlot'), + lockSource.indexOf('function', lockSource.indexOf('function acquireHookSlot') + 1), ); const mkdirCatch = slotFn.slice( slotFn.indexOf('fs.mkdirSync(lockDir'), @@ -591,6 +606,7 @@ describe('Cursor integration install docs', () => { const body = fs.readFileSync(integrationReadme, 'utf-8'); expect(body).toContain('.cursor/hooks.json'); expect(body).toContain('hooks/gitnexus-hook.cjs'); + expect(body).toContain('hooks/hook-lock.cjs'); expect(body).toContain('Hook install'); }); diff --git a/gitnexus/test/unit/hooks.test.ts b/gitnexus/test/unit/hooks.test.ts index c3ca2a4a5..346ee1ed6 100644 --- a/gitnexus/test/unit/hooks.test.ts +++ b/gitnexus/test/unit/hooks.test.ts @@ -26,6 +26,7 @@ import { runHook, parseHookOutput } from '../utils/hook-test-helpers.js'; // ─── Paths to both hook variants ──────────────────────────────────── const CJS_HOOK = path.resolve(__dirname, '..', '..', 'hooks', 'claude', 'gitnexus-hook.cjs'); +const CJS_HOOK_LOCK = path.resolve(__dirname, '..', '..', 'hooks', 'claude', 'hook-lock.cjs'); const PLUGIN_HOOK = path.resolve( __dirname, '..', @@ -35,6 +36,15 @@ const PLUGIN_HOOK = path.resolve( 'hooks', 'gitnexus-hook.js', ); +const PLUGIN_HOOK_LOCK = path.resolve( + __dirname, + '..', + '..', + '..', + 'gitnexus-claude-plugin', + 'hooks', + 'hook-lock.js', +); // ─── Test fixtures: temporary .gitnexus directory ─────────────────── @@ -297,12 +307,18 @@ 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], + for (const [label, hookPath, lockPath] of [ + ['CJS', CJS_HOOK, CJS_HOOK_LOCK], + ['Plugin', PLUGIN_HOOK, PLUGIN_HOOK_LOCK], ] as const) { - it(`${label} hook defines acquireHookSlot`, () => { + it(`${label} hook loads acquireHookSlot helper`, () => { const source = fs.readFileSync(hookPath, 'utf-8'); + expect(source).toContain('acquireHookSlot'); + expect(source).toContain('hook-lock'); + }); + + it(`${label} helper defines acquireHookSlot`, () => { + const source = fs.readFileSync(lockPath, 'utf-8'); expect(source).toContain('function acquireHookSlot'); expect(source).toContain('HOOK_LOCK_MAX_INFLIGHT'); }); @@ -322,7 +338,7 @@ describe('PreToolUse concurrency guard', () => { // entries then wrote a per-pid lock, which let simultaneous bursts // exceed MAX_INFLIGHT. The hard-cap version writes to fixed-name // slot-N.lock paths so O_CREAT|O_EXCL is atomic across processes. - const source = fs.readFileSync(hookPath, 'utf-8'); + const source = fs.readFileSync(lockPath, 'utf-8'); expect(source).toMatch(/slot-\$\{slot\}\.lock|`slot-/); // And no longer reads the lock dir to count active hooks. const slotFn = source.slice( @@ -337,7 +353,7 @@ describe('PreToolUse concurrency guard', () => { // 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 source = fs.readFileSync(lockPath, 'utf-8'); const slotFn = source.slice( source.indexOf('function acquireHookSlot'), source.indexOf('function', source.indexOf('function acquireHookSlot') + 1),