mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-07 08:26:11 +00:00
ec4624af87
* 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> * fix(hooks): make augment concurrency cap a hard cap via atomic slot files Address Claude's review of #1510. The original count-then-claim guard had a TOCTOU window: N hooks could each read `active < MAX_INFLIGHT` between readdirSync and the per-pid `wx` write and all proceed, briefly exceeding the cap. The PR title's "cap" language overstated this. Replace with fixed-name `slot-0.lock` ... `slot-N.lock` under `.hook-locks/`. `O_CREAT|O_EXCL` on a fixed path is OS-atomic — exactly one process wins each slot, so the cap is hard regardless of burst arrival timing. Each slot file contains the owning PID so stale-takeover still works when a hook crashes without releasing. PID liveness is checked before age (Claude's Finding 3): a slow-but-alive hook is never wrongly evicted. The 30s age window only kicks in to defend against PID reuse on a long-abandoned slot, well above the 7s augment timeout so a healthy run never hits it. Also adds the missing concurrency-guard tests to cursor-hook.test.ts (Claude's Finding 2): source-level wiring + dead-PID reclaim + 3-slots-full bail. Previously only the CJS and Plugin variants had test coverage for the guard; the Cursor variant was validated only by code inspection. Tests: 5726 passing, +9 from baseline (1 hard-cap burst test + 4 source regressions in hooks.test.ts; 3 source + 2 integration in cursor-hook.test.ts). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hooks): inspect slot mtime + content via single fd (codeql TOCTOU) CodeQL flagged the stale-takeover path in acquireHookSlot as a potential filesystem race (js/file-system-race): statSync(slotPath) followed by readFileSync(slotPath) gives a TOCTOU window where the file could be swapped between the metadata check and the content read. Replace the two separate path-based calls with a single openSync + fstatSync + readSync + closeSync sequence. Both mtime and owner PID now come from the same file descriptor, so the operations are atomic on one inode. No behavioral change beyond closing the race. Applied to all three hook variants (CJS, Plugin, Cursor). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hooks): distinguish EPERM from ESRCH in PID liveness check Cursor Bugbot caught a contradiction with the stated design: the bare `catch` after `process.kill(owner, 0)` was treating EPERM (process exists but owned by another user) the same as ESRCH (process gone), which would evict a live slot whenever the lock dir straddled user boundaries. Inspect the error code: ESRCH → dead, evict; EPERM → still alive, keep the slot; anything else → assume alive (be conservative under unexpected failure rather than over-evict). Applied to all three hook variants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hooks): fail closed when lock dir cannot be created Previously the mkdirSync catch in acquireHookSlot returned `() => {}` (a truthy no-op). The caller checks `if (!release) return;` to skip augment when the guard can't be established — but a truthy no-op slipped through that check and let augment spawn unguarded. On a cross-user shared `.gitnexus/` or read-only filesystem, N concurrent hooks would each take that branch and reintroduce the #1486 fan-out the guard exists to prevent. Return `null` instead so the caller's `if (!release) return;` skips augment cleanly. Augment is best-effort enrichment — skipping it when the guard fails is strictly safer than running unguarded. Also clarify the stale-slot comment: PID-liveness wins for slots younger than HOOK_LOCK_STALE_MS, but age is the final arbiter beyond 30s (PID-reuse defense). The previous wording said "PID-liveness wins over age" without qualifying it, which contradicted the >30s branch. Add source-level regression tests in hooks.test.ts and cursor-hook.test.ts asserting acquireHookSlot returns null (not () => {}) on lock-dir failure. Note in the Cursor test file that the 10-spawner burst test is not duplicated because the algorithm is byte-for-byte identical to the CJS hook and already covered there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(hooks): extract lock guard into helper modules Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/04dd20c5-28fd-433a-83cf-ad83fd03fb32 --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|---|---|---|
| .. | ||
| gitnexus-hook.cjs | ||
| hook-lock.cjs | ||
| hooks.json | ||