fix(hooks): wrap the augment CLI child in the orphan guard (#2163)

CHANGELOG.md

@@ -7,6 +7,7 @@ All notable changes to GitNexus will be documented in this file.
 ### Fixed
 
 - **Hook db-lock probe no longer strands unkillable `lsof`/`ps` orphans** — the probe's `lsof`/`ps` subprocesses are now wrapped in a self-tested coreutils `timeout`/`gtimeout` (`timeout -k 1 …`), so a hook SIGKILLed by the runner's 10s timeout can no longer leave `lsof` running forever (orphan lifetime bounded at ~3s); `acquireHookSlot` now also gates the probe itself, capping concurrent probes at 3 per repo. Opt out with `GITNEXUS_HOOK_TIMEOUT_PATH=disabled`. (#2163)
+- **Hook augment CLI no longer strands orphans either** — `runGitNexusCli` in the Claude, plugin, and Antigravity hook adapters now wraps the `gitnexus augment` subprocess (the longest-lived hook child: 7s local / 12s npx inner budgets) in the same self-tested coreutils `timeout` guard as the probe's `lsof`/`ps`, with a budget of ceil(inner/1000)+1 seconds — strictly above the inner `spawnSync` timeout, so on the supervised path Node's SIGTERM still fires first and observable behavior is unchanged. Once the hook itself has been SIGKILLed the guard takes over, with per-branch semantics: on the direct-exec branches (the CLI is the guard's child) it SIGTERMs at budget and `-k 1` SIGKILLs 1s later; on the npx branches (the CLI is a *grandchild* behind npx) it uses `-s KILL`, SIGKILLing the whole process group at budget — a TERM-first guard there would only kill the obedient npx parent and exit before its `-k` escalation fires, stranding a SIGTERM-immune CLI. Two npx-branch caveats remain (both no worse than the pre-fix behavior, where the grandchild received no signal at all): the group-wide KILL is coreutils semantics, so a busybox `timeout` — which passes the self-test — still signals only its direct child and cannot reach the grandchild; and on the supervised path (hook alive, inner `spawnSync` timeout SIGTERMs the guard) coreutils forwards TERM rather than KILL, so a SIGTERM-immune CLI grandchild is still not reaped there. The guard self-test now also requires exit-status propagation (`sh -c 'exit 42'` must yield 42), so an always-exit-0 stub at `GITNEXUS_HOOK_TIMEOUT_PATH` can no longer be adopted and silently swallow the probe and augment. Windows, `GITNEXUS_HOOK_TIMEOUT_PATH=disabled`, and Unix hosts with no usable coreutils `timeout`/`gtimeout` at all (e.g. macOS without Homebrew coreutils) keep the exact pre-wrap unguarded invocation — every guard-less Unix run, whatever the reason (disabled, nothing usable, probe version skew), is now diagnosed once per hook run under `GITNEXUS_DEBUG`. The Cursor hook is not wrapped yet (it does not install the probe helper) but now reports its slot-saturated skip under `GITNEXUS_DEBUG`. (#2163 follow-up)
 
 ### Changed
 - Migrated from KuzuDB to LadybugDB v0.15 (`@ladybugdb/core`, `@ladybugdb/wasm-core`)

gitnexus-claude-plugin/hooks/gitnexus-hook.js

@@ -15,7 +15,10 @@ const fs = require('fs');
 const path = require('path');
 const { spawnSync } = require('child_process');
 const { acquireHookSlot } = require('./hook-lock.js');
-const { hasGitNexusDbLockedByGitNexusServer } = require('./hook-db-lock-probe.cjs');
+const {
+  hasGitNexusDbLockedByGitNexusServer,
+  resolveUnixGuardTimeout,
+} = require('./hook-db-lock-probe.cjs');
 const { formatAnalyzeCommand } = require('./resolve-analyze-cmd.cjs');
 
 /**
@@ -196,18 +199,90 @@ function extractPattern(toolName, toolInput) {
   return null;
 }
 
+// Debounce for the unguarded-CLI diagnostic below (#2163 follow-up review):
+// at most one line per (short-lived) hook process, even if a future change
+// runs the CLI more than once.
+let unguardedCliWarned = false;
+
 /**
  * Spawn a gitnexus CLI command synchronously.
  * Detects binary on PATH once, then runs exactly once.
  *
  * SECURITY: Never use shell: true with user-controlled arguments.
  * On Windows, invoke gitnexus.cmd directly (no shell needed).
+ *
+ * Unix orphan containment (#2163 follow-up): the augment CLI is the
+ * longest-lived hook child (inner spawnSync timeout 7s locally, 12s via
+ * npx), so on Unix every CLI-running branch gets the same SIGKILL-surviving
+ * coreutils `timeout` wrapper as the probe's lsof/ps (the cheap which/where
+ * PATH check stays unwrapped). The wrapper budget is ceil(inner/1000)+1
+ * seconds — STRICTLY greater than the inner spawnSync timeout, so on the
+ * supervised path Node's SIGTERM always fires first and the existing
+ * error/status contract is untouched. Once the hook itself has been
+ * SIGKILLed (exactly the orphan case the wrapper exists for), the guard
+ * semantics differ per branch:
+ *   - direct exec (GITNEXUS_HOOK_CLI_PATH / PATH-installed `gitnexus`; the
+ *     CLI is the guard's CHILD): `-k 1` TERM-first — a SIGTERM-immune CLI
+ *     can hold the guard ~1s past the inner timeout before the `-k` SIGKILL
+ *     escalation reaps it.
+ *   - npx (the CLI is a GRANDCHILD: guard → npx → CLI): `-s KILL` — the
+ *     budget expiry SIGKILLs the whole process group outright. TERM-first
+ *     would kill only the obedient npx parent, making `timeout` reap it and
+ *     return before the `-k` escalation ever fires, stranding a
+ *     SIGTERM-immune CLI grandchild unbounded (reproduced on coreutils
+ *     9.x). `-k 1` is retained alongside `-s KILL` as a harmless belt: with
+ *     `-s KILL` the `-k` escalation signal is also KILL. Two residual gaps
+ *     on this branch, both bounded by "no worse than pre-fix" (where the
+ *     grandchild received no signal at all): the group-wide SIGKILL is
+ *     coreutils semantics — a busybox `timeout` passes the self-test (it
+ *     has `-k` and propagates exit status) but signals only its direct
+ *     child, so a busybox guard cannot reach the grandchild; and on the
+ *     SUPERVISED path (hook alive, inner spawnSync timeout SIGTERMs the
+ *     guard) coreutils forwards TERM rather than the `-s` signal, npx dies,
+ *     and the guard exits before any KILL fires — so a SIGTERM-immune CLI
+ *     grandchild still escapes in those two cases.
+ * If the sibling probe predates the resolveUnixGuardTimeout export (version
+ * skew), the adapter degrades to the unwrapped invocation instead of
+ * throwing. Windows is deliberately NOT wrapped — there is no coreutils
+ * timeout to resolve there and the resolver's self-test spawns /bin/sh — so
+ * on win32 (the gitnexus.cmd / npx.cmd paths) and whenever the guard
+ * resolves to null (e.g. macOS without Homebrew coreutils — reported once
+ * under GITNEXUS_DEBUG) the argv stays byte-identical to the pre-wrap
+ * invocation.
  */
 function runGitNexusCli(args, cwd, timeout) {
   const isWin = process.platform === 'win32';
+  // Version-skew guard (#2163 follow-up review): an older sibling probe
+  // without the resolveUnixGuardTimeout export must degrade to the unwrapped
+  // invocation — a TypeError here would be swallowed by the caller's catch
+  // and silently kill the augment.
+  const guard =
+    isWin || typeof resolveUnixGuardTimeout !== 'function' ? null : resolveUnixGuardTimeout();
+  if (!isWin && !guard && !unguardedCliWarned && isDebugEnabled()) {
+    // Diagnose the "stays unwrapped" Unix paths once per hook process: no
+    // usable coreutils timeout/gtimeout (e.g. macOS without Homebrew
+    // coreutils), GITNEXUS_HOOK_TIMEOUT_PATH=disabled, or probe skew above.
+    unguardedCliWarned = true;
+    process.stderr.write(
+      '[GitNexus hook] no usable timeout/gtimeout guard; augment CLI child runs unguarded\n',
+    );
+  }
   const hookCli = process.env.GITNEXUS_HOOK_CLI_PATH;
   if (hookCli !== undefined && String(hookCli).trim() && fs.existsSync(String(hookCli))) {
-    return spawnSync(process.execPath, [String(hookCli), ...args], {
+    const [cmd, cmdArgs] = guard
+      ? [
+          guard,
+          [
+            '-k',
+            '1',
+            String(Math.ceil(timeout / 1000) + 1),
+            process.execPath,
+            String(hookCli),
+            ...args,
+          ],
+        ]
+      : [process.execPath, [String(hookCli), ...args]];
+    return spawnSync(cmd, cmdArgs, {
       encoding: 'utf-8',
       timeout,
       cwd,
@@ -231,16 +306,40 @@ function runGitNexusCli(args, cwd, timeout) {
   }
 
   if (useDirectBinary) {
-    return spawnSync(isWin ? 'gitnexus.cmd' : 'gitnexus', args, {
+    // A non-null guard implies non-Windows, so the wrapped arm can hardcode
+    // plain `gitnexus` (the guard resolves it via PATH, like spawnSync does).
+    const [cmd, cmdArgs] = guard
+      ? [guard, ['-k', '1', String(Math.ceil(timeout / 1000) + 1), 'gitnexus', ...args]]
+      : [isWin ? 'gitnexus.cmd' : 'gitnexus', args];
+    return spawnSync(cmd, cmdArgs, {
       encoding: 'utf-8',
       timeout,
       cwd,
       stdio: ['pipe', 'pipe', 'pipe'],
       windowsHide: true,
     });
   }
-  // npx fallback needs shell on Windows since npx is a .cmd script
-  return spawnSync(isWin ? 'npx.cmd' : 'npx', ['-y', 'gitnexus', ...args], {
+  // npx fallback needs shell on Windows since npx is a .cmd script. The
+  // wrapped arm leads with `-s KILL` (NOT TERM-first like the direct
+  // branches above): the CLI here is a grandchild behind npx — see the
+  // docblock.
+  const [cmd, cmdArgs] = guard
+    ? [
+        guard,
+        [
+          '-s',
+          'KILL',
+          '-k',
+          '1',
+          String(Math.ceil((timeout + 5000) / 1000) + 1),
+          'npx',
+          '-y',
+          'gitnexus',
+          ...args,
+        ],
+      ]
+    : [isWin ? 'npx.cmd' : 'npx', ['-y', 'gitnexus', ...args]];
+  return spawnSync(cmd, cmdArgs, {
     encoding: 'utf-8',
     timeout: timeout + 5000,
     cwd,

gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs

@@ -20,13 +20,18 @@
  *   it 1s later — orphan lifetime is bounded at ~3s instead of unbounded.
  * - GITNEXUS_HOOK_TIMEOUT_PATH: the sentinel value `disabled` switches the
  *   wrapper off deterministically; any other value is adopted only when it
- *   exists AND passes a one-shot `-k` self-test — otherwise resolution FALLS
- *   THROUGH to the built-in candidate list (first self-test pass wins), so
- *   no malformed value of any shape can silently disable orphan containment.
+ *   exists AND passes a one-shot `-k` exit-propagation self-test — otherwise
+ *   resolution FALLS THROUGH to the built-in candidate list (first self-test
+ *   pass wins), so no malformed value of any shape can silently disable
+ *   orphan containment.
  * - The gitnexus server is lazy-open + sticky-hold: an idle MCP server holds
  *   ZERO lbug fds until the repo's first MCP query, then keeps the fd open.
  *   A probe before that first query is therefore always false — a known,
  *   pre-existing race, not a bug in this probe.
+ * - resolveUnixGuardTimeout is exported so the hook adapters can wrap the
+ *   `gitnexus augment` CLI child — the longest-lived hook subprocess (7s
+ *   local / 12s npx inner budgets) — in the same guard; see runGitNexusCli
+ *   in the adapters (#2163 follow-up).
  */
 
 const fs = require('fs');
@@ -70,38 +75,53 @@ let unixGuardTimeoutCache;
 
 /**
  * Resolve a coreutils `timeout`/`gtimeout` binary to wrap lsof/ps with
- * (#2163). Dead code on Windows (the win32 dispatch returns earlier).
+ * (#2163). Unix-only by contract: the probe's win32 dispatch returns before
+ * reaching it, and the exported callers (the adapters' runGitNexusCli,
+ * #2163 follow-up) must check the platform first — the self-test below
+ * spawns /bin/sh. The memoized result is module-wide, so probe and adapter
+ * share one lazy self-test per hook process.
  *
  * GITNEXUS_HOOK_TIMEOUT_PATH semantics: the sentinel `disabled` turns the
  * wrapper off; any other value is only a CANDIDATE — an existing file path
- * is tried first, but it must pass the `-k` self-test to be adopted. On any
- * failure (non-existent path, directory, non-executable file, wrapper
- * without `-k` support, …) resolution falls through to the built-in
- * candidates below, tried in order, first self-test pass wins. This is
- * strictly stronger than the sibling GITNEXUS_HOOK_LSOF_PATH /
- * GITNEXUS_HOOK_PS_PATH overrides (which only check existence): no bad env
- * value of ANY shape can silently disable orphan containment.
+ * is tried first, but it must pass the `-k` exit-propagation self-test to
+ * be adopted. On any failure (non-existent path, directory, non-executable
+ * file, wrapper without `-k` support, always-exit-0 stub, …) resolution
+ * falls through to the built-in candidates below, tried in order, first
+ * self-test pass wins. This is strictly stronger than the sibling
+ * GITNEXUS_HOOK_LSOF_PATH / GITNEXUS_HOOK_PS_PATH overrides (which only
+ * check existence): no bad env value of ANY shape can silently disable
+ * orphan containment.
  *
  * Lazy self-test: candidates are probed only when the lsof/ps fallback is
  * first reached, and the result is memoized. A candidate is adopted only
- * when `timeout -k 1 1 /bin/sh -c :` exits 0. This rejects wrappers that do
- * not support the coreutils `-k` flag — busybox <1.34, toybox, broken
- * symlinks — which would otherwise exit with a usage error without ever
- * running lsof, silently converting the lsof-ETIMEDOUT fail-closed contract
- * into fail-open (#1492 regression). Only when EVERY candidate fails does
- * the probe fall back to the unwrapped status quo (memoized null).
- * busybox ≥1.34 passes the test and is fully usable (capability, not
- * identity, decides).
+ * when `timeout -k 1 1 /bin/sh -c 'exit 42'` exits 42 — i.e. it must RUN
+ * the wrapped command AND PROPAGATE its exit status. This rejects two
+ * failure shapes: wrappers without the coreutils `-k` flag — busybox <1.34,
+ * toybox, broken symlinks — which would exit with a usage error without
+ * ever running lsof, silently converting the lsof-ETIMEDOUT fail-closed
+ * contract into fail-open (#1492 regression); and always-exit-0 stubs
+ * (/bin/true shapes), which would otherwise be adopted and "succeed" every
+ * wrapped spawn instantly without running it — a constant no-owner probe
+ * answer and, worse, a silently dead augment (status 0, empty stderr passes
+ * the adapters' success check with no context; #2163 follow-up review).
+ * Only when EVERY candidate fails does the probe fall back to the unwrapped
+ * status quo (memoized null). busybox ≥1.34 passes the test and is fully
+ * usable for everything THIS file spawns (lsof/ps are the guard's direct
+ * children) and for the adapters' direct-exec arm. The adapters' npx arm
+ * additionally relies on coreutils' process-GROUP signalling for its
+ * `-s KILL` grandchild reaping; busybox signals only its direct child, and
+ * this self-test deliberately does not probe that capability — see the
+ * adapter docblocks for the residual-gap statement.
  */
 function passesGuardSelfTest(guard) {
   try {
-    const selfTest = spawnSync(guard, ['-k', '1', '1', '/bin/sh', '-c', ':'], {
+    const selfTest = spawnSync(guard, ['-k', '1', '1', '/bin/sh', '-c', 'exit 42'], {
       encoding: 'utf-8',
       timeout: 3000,
       stdio: ['ignore', 'ignore', 'ignore'],
       windowsHide: true,
     });
-    return !selfTest.error && selfTest.status === 0;
+    return !selfTest.error && selfTest.status === 42;
   } catch {
     return false;
   }
@@ -359,4 +379,16 @@ function hasGitNexusDbLockedByGitNexusServer(dbPath, myPid) {
 
 module.exports = {
   hasGitNexusDbLockedByGitNexusServer,
+  // #2163 follow-up: the hook adapters wrap the augment CLI in the same
+  // guard. Returns a self-tested wrapper path — the built-in candidates are
+  // always absolute; a GITNEXUS_HOOK_TIMEOUT_PATH override is adopted as the
+  // exact string that passed the self-test. Same string is also the same
+  // RESOLUTION for absolute paths and for slashless names (PATH lookup is
+  // cwd-independent); a slash-containing RELATIVE override, however, is
+  // existsSync-checked and self-tested against this process's cwd while the
+  // adapters spawn the CLI with a `cwd` option (chdir-before-exec), so such
+  // a value can pass here yet ENOENT at the augment call site — set the
+  // override to an absolute path. Returns null when the wrapper is
+  // disabled/unavailable. Never call on win32 (see its JSDoc).
+  resolveUnixGuardTimeout,
 };

gitnexus-cursor-integration/hooks/gitnexus-hook.cjs

@@ -241,7 +241,18 @@ function main() {
     if (!pattern || pattern.length < 3) return;
 
     const release = acquireHookSlot(gitNexusDir);
-    if (!release) return;
+    if (!release) {
+      // Normal skip path: all per-repo hook slots are held by concurrent
+      // sessions. Stays silent by default; surfaced only under the cursor
+      // hook's own GITNEXUS_DEBUG (truthy) convention. NOTE: unlike the
+      // claude/plugin/antigravity adapters this integration does not install
+      // hook-db-lock-probe.cjs, so its augment child is not guard-wrapped
+      // yet — tracked on the #2163 follow-up list ("cursor probe").
+      if (process.env.GITNEXUS_DEBUG) {
+        process.stderr.write('[GitNexus] augment skipped: hook slots saturated\n');
+      }
+      return;
+    }
 
     const cliPath = resolveCliPath();
     let result = '';