diff --git a/gitnexus-claude-plugin/hooks/gitnexus-hook.js b/gitnexus-claude-plugin/hooks/gitnexus-hook.js index 021dce60c..a53d79f29 100644 --- a/gitnexus-claude-plugin/hooks/gitnexus-hook.js +++ b/gitnexus-claude-plugin/hooks/gitnexus-hook.js @@ -381,6 +381,49 @@ function sendHookResponse(hookEventName, message) { ); } +/** + * Fallback augmentation for the #2396 path: when a GitNexus process holds the + * lbug DB write lock the CLI `augment` can't run, so point the agent at the MCP + * `query` tool instead. Phrased conditionally ("if the MCP tools are live") so it + * stays truthful on every owner path — a confirmed MCP owner, a `serve` owner, or + * a fail-closed probe where no server is actually confirmed. `pattern` is embedded + * verbatim; the caller (sendHookResponse) JSON-escapes it structurally. + */ +function buildMcpQueryHint(pattern) { + return ( + `[GitNexus] Local augment is unavailable (the graph DB is held by another ` + + `GitNexus process). If the GitNexus MCP tools are live in this session, call ` + + `the GitNexus \`query\` MCP tool (e.g. mcp__gitnexus__query) with ` + + `search_query "${pattern}".` + ); +} + +/** + * #2396 throttle: emit the MCP-query hint at most once per repo per window, so an + * owner-locked session isn't nudged on every search. Window (ms) via + * GITNEXUS_MCP_HINT_THROTTLE_MS (default 10min; 0/invalid disables). Best-effort — + * any fs error falls back to emitting. + * ponytail: per-repo mtime marker, shared across concurrent sessions on the same + * repo; add per-session dedup only if that sharing becomes a problem. + */ +function shouldEmitMcpHint(gitNexusDir) { + const raw = process.env.GITNEXUS_MCP_HINT_THROTTLE_MS; + const windowMs = raw === undefined || raw === '' ? 600000 : Number(raw); + if (!Number.isFinite(windowMs) || windowMs <= 0) return true; + const marker = path.join(gitNexusDir, '.mcp-hint-shown'); + try { + if (Date.now() - fs.statSync(marker).mtimeMs < windowMs) return false; + } catch { + /* marker missing/unreadable → emit */ + } + try { + fs.writeFileSync(marker, ''); + } catch { + /* best-effort; still emit */ + } + return true; +} + /** * PreToolUse handler — augment searches with graph context. */ @@ -417,17 +460,24 @@ function handlePreToolUse(input) { let result = ''; try { if (hasGitNexusServerOwner(gitNexusDir)) { - // Normal skip path: the MCP server owns the DB, so the CLI augment would - // contend on the lock. Stay silent for strict hook runners (issue #1913); - // surface the reason only when diagnostics are explicitly requested. + // #2396: the MCP server holds the DB write lock, so a competing CLI + // `augment` would only contend on it (LadybugDB is single-writer). But the + // session that triggered this hook has the GitNexus MCP tools live — route + // the augmentation to the agent via additionalContext instead of silently + // doing nothing. Mirror the skip reason to stderr only under GITNEXUS_DEBUG + // (strict-runner contract, #1913); the hint itself rides the sanctioned + // additionalContext stdout channel the successful augment already uses. if (isDebugEnabled()) { process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); } - return; - } - const child = runGitNexusCli(['augment', '--', pattern], cwd, 7000); - if (!child.error && child.status === 0) { - result = extractAugmentContext(child.stderr || ''); + if (shouldEmitMcpHint(gitNexusDir)) { + result = buildMcpQueryHint(pattern); + } + } else { + const child = runGitNexusCli(['augment', '--', pattern], cwd, 7000); + if (!child.error && child.status === 0) { + result = extractAugmentContext(child.stderr || ''); + } } } catch { /* graceful failure */ diff --git a/gitnexus/README.md b/gitnexus/README.md index 67e9d39a8..9f88a954e 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -535,17 +535,26 @@ After scope resolution, analyze prunes inert block-local value symbols (a functi Programmatic callers can pass `keepLocalValueSymbols: true` in `PipelineOptions` instead of setting the env var. -### Hook augmentation/notifications are silently skipped +### Hook augmentation and skip diagnostics -The Claude Code / Antigravity hooks intentionally stay **silent** on normal skip +The Claude Code / Antigravity hooks keep their **stderr** silent on normal skip paths so strict hook runners (e.g. Codex `PreToolUse`) never see unexpected -output. A search may not be augmented — or a stale-index reminder may not appear -on stderr — when the GitNexus MCP server owns the repo DB, when the DB-lock probe -times out and fails closed, or when the index is already current. +diagnostic output. -To see why a hook skipped, set `GITNEXUS_DEBUG=1` and re-run the action — the hook -writes the reason (e.g. `[GitNexus] augment skipped: MCP server owns DB`) and the -stale-index hint to its stderr: +When a GitNexus process holds the repo DB write lock (the common case — the MCP +server is running, or the DB-lock probe timed out and failed closed), the local +CLI `augment` can't run (LadybugDB is single-writer). Rather than drop the +augmentation, the hook hands the agent a short, conditional MCP-query hint on +stdout (the sanctioned `additionalContext` channel) — _"if the GitNexus MCP tools +are live in this session, call `query` …"_ — so an agent that has the tools can +still fetch graph-ranked context. The hint is throttled to at most once per repo +per window (`GITNEXUS_MCP_HINT_THROTTLE_MS`, default 10 min; `0` disables), so an +owner-locked session isn't nudged on every search. A stale-index reminder, or an +already-current index, stays silent. + +To see why a hook skipped the CLI augment, set `GITNEXUS_DEBUG=1` and re-run the +action — the hook writes the reason (e.g. `[GitNexus] augment skipped: MCP server +owns DB`) and the stale-index hint to its stderr: ```bash GITNEXUS_DEBUG=1 # surfaces hook skip/diagnostic reasons on stderr diff --git a/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs b/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs index 6b9e8b9f8..3331ae3fc 100755 --- a/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs +++ b/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs @@ -391,6 +391,49 @@ function buildAfterToolContext(input) { return parts.length > 0 ? parts.join('\n\n') : null; } +/** + * Fallback augmentation for the #2396 path: when a GitNexus process holds the + * lbug DB write lock the CLI `augment` can't run, so point the agent at the MCP + * `query` tool instead. Phrased conditionally ("if the MCP tools are live") so it + * stays truthful on every owner path — a confirmed MCP owner, a `serve` owner, or + * a fail-closed probe where no server is actually confirmed. `pattern` is embedded + * verbatim; the caller (writeAdditionalContext) JSON-escapes it structurally. + */ +function buildMcpQueryHint(pattern) { + return ( + `[GitNexus] Local augment is unavailable (the graph DB is held by another ` + + `GitNexus process). If the GitNexus MCP tools are live in this session, call ` + + `the GitNexus \`query\` MCP tool (e.g. mcp__gitnexus__query) with ` + + `search_query "${pattern}".` + ); +} + +/** + * #2396 throttle: emit the MCP-query hint at most once per repo per window, so an + * owner-locked session isn't nudged on every search. Window (ms) via + * GITNEXUS_MCP_HINT_THROTTLE_MS (default 10min; 0/invalid disables). Best-effort — + * any fs error falls back to emitting. + * ponytail: per-repo mtime marker, shared across concurrent sessions on the same + * repo; add per-session dedup only if that sharing becomes a problem. + */ +function shouldEmitMcpHint(gitNexusDir) { + const raw = process.env.GITNEXUS_MCP_HINT_THROTTLE_MS; + const windowMs = raw === undefined || raw === '' ? 600000 : Number(raw); + if (!Number.isFinite(windowMs) || windowMs <= 0) return true; + const marker = path.join(gitNexusDir, '.mcp-hint-shown'); + try { + if (Date.now() - fs.statSync(marker).mtimeMs < windowMs) return false; + } catch { + /* marker missing/unreadable → emit */ + } + try { + fs.writeFileSync(marker, ''); + } catch { + /* best-effort; still emit */ + } + return true; +} + function runAugment(gitNexusDir, cwd, pattern) { // Acquire the per-repo slot BEFORE the DB-owner probe (#2163): the probe // itself spawns lsof/ps, so it must be bounded by the same ≤3-per-repo cap @@ -410,12 +453,16 @@ function runAugment(gitNexusDir, cwd, pattern) { } try { if (hasGitNexusServerOwner(gitNexusDir)) { - // Normal skip path: the MCP server owns the DB. Stay silent for strict - // hook runners (issue #1913); surface the reason only under GITNEXUS_DEBUG. + // #2396: the MCP server holds the DB write lock, so a competing CLI + // `augment` would only contend on it (LadybugDB is single-writer). The + // session has the GitNexus MCP tools live — route the augmentation to the + // agent via additionalContext instead of dropping it. Mirror the skip + // reason to stderr only under GITNEXUS_DEBUG (strict-runner contract, + // #1913); the hint itself rides the sanctioned additionalContext channel. if (isDebugEnabled()) { process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); } - return ''; + return shouldEmitMcpHint(gitNexusDir) ? buildMcpQueryHint(pattern) : ''; } const cliPath = resolveCliPath(); const child = runGitNexusCli(cliPath, ['augment', '--', pattern], cwd, 7000); diff --git a/gitnexus/hooks/claude/gitnexus-hook.cjs b/gitnexus/hooks/claude/gitnexus-hook.cjs index 2f24a6f4d..18be614f4 100755 --- a/gitnexus/hooks/claude/gitnexus-hook.cjs +++ b/gitnexus/hooks/claude/gitnexus-hook.cjs @@ -349,6 +349,49 @@ function runGitNexusCli(cliPath, args, cwd, timeout) { }); } +/** + * Fallback augmentation for the #2396 path: when a GitNexus process holds the + * lbug DB write lock the CLI `augment` can't run, so point the agent at the MCP + * `query` tool instead. Phrased conditionally ("if the MCP tools are live") so it + * stays truthful on every owner path — a confirmed MCP owner, a `serve` owner, or + * a fail-closed probe where no server is actually confirmed. `pattern` is embedded + * verbatim; the caller (sendHookResponse) JSON-escapes it structurally. + */ +function buildMcpQueryHint(pattern) { + return ( + `[GitNexus] Local augment is unavailable (the graph DB is held by another ` + + `GitNexus process). If the GitNexus MCP tools are live in this session, call ` + + `the GitNexus \`query\` MCP tool (e.g. mcp__gitnexus__query) with ` + + `search_query "${pattern}".` + ); +} + +/** + * #2396 throttle: emit the MCP-query hint at most once per repo per window, so an + * owner-locked session isn't nudged on every search. Window (ms) via + * GITNEXUS_MCP_HINT_THROTTLE_MS (default 10min; 0/invalid disables). Best-effort — + * any fs error falls back to emitting. + * ponytail: per-repo mtime marker, shared across concurrent sessions on the same + * repo; add per-session dedup only if that sharing becomes a problem. + */ +function shouldEmitMcpHint(gitNexusDir) { + const raw = process.env.GITNEXUS_MCP_HINT_THROTTLE_MS; + const windowMs = raw === undefined || raw === '' ? 600000 : Number(raw); + if (!Number.isFinite(windowMs) || windowMs <= 0) return true; + const marker = path.join(gitNexusDir, '.mcp-hint-shown'); + try { + if (Date.now() - fs.statSync(marker).mtimeMs < windowMs) return false; + } catch { + /* marker missing/unreadable → emit */ + } + try { + fs.writeFileSync(marker, ''); + } catch { + /* best-effort; still emit */ + } + return true; +} + /** * PreToolUse handler — augment searches with graph context. */ @@ -385,18 +428,25 @@ function handlePreToolUse(input) { let result = ''; try { if (hasGitNexusServerOwner(gitNexusDir)) { - // Normal skip path: the MCP server owns the DB, so the CLI augment would - // contend on the lock. Stay silent for strict hook runners (issue #1913); - // surface the reason only when diagnostics are explicitly requested. + // #2396: the MCP server holds the DB write lock, so a competing CLI + // `augment` would only contend on it (LadybugDB is single-writer). But the + // session that triggered this hook has the GitNexus MCP tools live — route + // the augmentation to the agent via additionalContext instead of silently + // doing nothing. Mirror the skip reason to stderr only under GITNEXUS_DEBUG + // (strict-runner contract, #1913); the hint itself rides the sanctioned + // additionalContext stdout channel the successful augment already uses. if (isDebugEnabled()) { process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); } - return; - } - const cliPath = resolveCliPath(); - const child = runGitNexusCli(cliPath, ['augment', '--', pattern], cwd, 7000); - if (!child.error && child.status === 0) { - result = extractAugmentContext(child.stderr || ''); + if (shouldEmitMcpHint(gitNexusDir)) { + result = buildMcpQueryHint(pattern); + } + } else { + const cliPath = resolveCliPath(); + const child = runGitNexusCli(cliPath, ['augment', '--', pattern], cwd, 7000); + if (!child.error && child.status === 0) { + result = extractAugmentContext(child.stderr || ''); + } } } catch { /* graceful failure */ diff --git a/gitnexus/test/integration/antigravity-hook-e2e.test.ts b/gitnexus/test/integration/antigravity-hook-e2e.test.ts index b99005a91..98e64fbce 100644 --- a/gitnexus/test/integration/antigravity-hook-e2e.test.ts +++ b/gitnexus/test/integration/antigravity-hook-e2e.test.ts @@ -416,14 +416,16 @@ describe('antigravity hook adapter e2e', () => { }); }); - // Issue #1913: when a GitNexus MCP server owns the repo DB, runAugment() must - // SKIP — silently by default so strict hook runners never see unexpected - // output, and surface the reason only under GITNEXUS_DEBUG=1. The Claude/Plugin - // copies are covered in test/unit/hooks.test.ts; the antigravity adapter shares - // the identical gated skip and is exercised here through the install pipeline - // (its lock/probe helpers only resolve from the install dir). A faked lsof/ps + - // an empty `lbug` lock force hasGitNexusServerOwner() => true; a marker-writing - // fake CLI proves augment never ran. + // #2396: when a GitNexus MCP server owns the repo DB, runAugment() cannot run + // the CLI augment (LadybugDB is single-writer), so it returns an MCP-query hint + // that reaches the agent via additionalContext instead of dropping the + // augmentation. #1913: the stderr skip diagnostic stays gated behind + // GITNEXUS_DEBUG=1. The Claude/Plugin copies are covered in + // test/unit/hooks.test.ts; the antigravity adapter shares the identical path + // and is exercised here through the install pipeline (its lock/probe helpers + // only resolve from the install dir). A faked lsof/ps + an empty `lbug` lock + // force hasGitNexusServerOwner() => true; a marker-writing fake CLI proves the + // CLI augment never ran. // // #2180: skipped on Linux too — the probe's Linux backend no longer uses // lsof/ps, so the faked lsof/ps can't force owner=true there. This stays as the @@ -431,14 +433,14 @@ describe('antigravity hook adapter e2e', () => { // gated owner-skip with the claude/plugin copies, whose Linux owner detection // is covered against a fake /proc in test/unit/hook-db-lock-probe.test.ts. describe.skipIf(process.platform === 'win32' || process.platform === 'linux')( - 'AfterTool — augment skipped when MCP server owns the DB (#1913)', + 'AfterTool — MCP-query hint when MCP server owns the DB (#2396)', () => { const OWNER_PROBE = { lsofOutput: '12345\n', psOutput: 'node /tmp/node_modules/.bin/gitnexus mcp\n', }; - it('stays SILENT by default (no augment ran, no stderr noise, exit 0)', () => { + it('emits the MCP-query hint on stdout, no stderr noise, exit 0 (CLI augment never ran)', () => { const markerPath = path.join(os.tmpdir(), `antigravity-skip-silent-${process.pid}`); const lbugPath = path.join(gitNexusDir, 'lbug'); fs.writeFileSync(lbugPath, ''); @@ -459,13 +461,15 @@ describe('antigravity hook adapter e2e', () => { ); expect(result.status).toBe(0); - // Strict-runner contract: completely silent — empty stdout AND stderr - // (matches the unit suite's assertion strength for the claude/plugin copies). - expect(result.stdout.trim()).toBe(''); + // #2396: the augmentation is handed to the agent as an MCP-query hint on + // stdout; stderr stays silent (strict-runner contract, #1913). + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); + expect(output!.additionalContext).toContain('validateUser'); expect(result.stderr.trim()).toBe(''); - // Marker absent ⇒ the CLI never ran (augment short-circuited at the owner - // check). The paired GITNEXUS_DEBUG=1 test below positively proves the skip - // was the owner path (it asserts the owner-skip diagnostic on stderr). + // Marker absent ⇒ the CLI never ran (short-circuited at the owner check). + // The paired GITNEXUS_DEBUG=1 test below positively proves the path was + // the owner path (it asserts the owner-skip diagnostic on stderr). expect(fs.existsSync(markerPath)).toBe(false); } finally { fs.rmSync(lbugPath, { force: true }); @@ -495,7 +499,10 @@ describe('antigravity hook adapter e2e', () => { ); expect(result.status).toBe(0); - expect(parseHookOutput(result.stdout)).toBeNull(); + // The hint still rides stdout; GITNEXUS_DEBUG only adds the stderr reason. + expect(parseHookOutput(result.stdout)!.additionalContext).toContain( + 'mcp__gitnexus__query', + ); expect(result.stderr).toContain('[GitNexus] augment skipped: MCP server owns DB'); expect(fs.existsSync(markerPath)).toBe(false); } finally { diff --git a/gitnexus/test/unit/hooks.test.ts b/gitnexus/test/unit/hooks.test.ts index afc4c2f3d..82eeb19f9 100644 --- a/gitnexus/test/unit/hooks.test.ts +++ b/gitnexus/test/unit/hooks.test.ts @@ -1817,13 +1817,14 @@ describe('PreToolUse augmentation filtering (integration)', () => { } }); - // Issue #1913: the MCP-owned-DB skip is a NORMAL (non-error) path, so by - // default it must stay completely silent — empty stdout AND empty stderr, - // exit 0 — so strict hook runners (e.g. Codex `PreToolUse`) never see - // unexpected output. GITNEXUS_DEBUG is forced off to keep the assertion - // deterministic regardless of the ambient environment. + // #2396: when a GitNexus MCP process owns the repo DB the CLI augment can't + // run, so the hook hands the agent the MCP-query hint on stdout (the sanctioned + // additionalContext channel). By default (GITNEXUS_DEBUG unset) the stderr skip + // diagnostic stays silent, so strict hook runners (e.g. Codex `PreToolUse`) see + // no unexpected diagnostic noise — only the augmentation itself (#1913). This is + // the GITNEXUS_DEBUG='' owner-hint coverage; the debug variants are below. it.skipIf(SKIP_LSOF_PATH)( - `${label}: skips augment SILENTLY when a GitNexus MCP process owns the repo DB`, + `${label}: emits the MCP-query hint on stdout, stderr silent by default, when a GitNexus MCP process owns the repo DB`, () => { const markerPath = path.join(os.tmpdir(), `gitnexus-hook-called-${process.pid}-${label}`); const lbugPath = path.join(gitNexusDir, 'lbug'); @@ -1847,7 +1848,9 @@ describe('PreToolUse augmentation filtering (integration)', () => { { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '' } }, ); - expect(result.stdout.trim()).toBe(''); + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); + expect(output!.additionalContext).toContain('validateUser'); expect(result.stderr.trim()).toBe(''); expect(result.status).toBe(0); expect(fs.existsSync(markerPath)).toBe(false); @@ -1859,11 +1862,13 @@ describe('PreToolUse augmentation filtering (integration)', () => { }, ); - // Issue #1913: the skip reason remains recoverable for operators who opt in - // via GITNEXUS_DEBUG=1 — stdout stays empty (no augment ran), the diagnostic - // appears on stderr. + // #2396: when the MCP server owns the DB the CLI augment can't run, so the + // hook hands the agent an MCP-query hint on stdout (the sanctioned + // additionalContext channel) instead of doing nothing. The CLI still never + // spawns (marker absent). #1913: the stderr skip diagnostic stays gated + // behind GITNEXUS_DEBUG. it.skipIf(SKIP_LSOF_PATH)( - `${label}: surfaces the MCP-owner skip reason only under GITNEXUS_DEBUG`, + `${label}: MCP-owner path emits the MCP query hint; stderr reason gated by GITNEXUS_DEBUG`, () => { const markerPath = path.join(os.tmpdir(), `gitnexus-hook-dbg-${process.pid}-${label}`); const lbugPath = path.join(gitNexusDir, 'lbug'); @@ -1887,7 +1892,9 @@ describe('PreToolUse augmentation filtering (integration)', () => { { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); - expect(result.stdout.trim()).toBe(''); + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); + expect(output!.additionalContext).toContain('validateUser'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped: MCP server owns DB'); expect(fs.existsSync(markerPath)).toBe(false); @@ -1900,13 +1907,13 @@ describe('PreToolUse augmentation filtering (integration)', () => { ); // #1913: the GITNEXUS_DEBUG contract is strict — ONLY '1' and 'true' enable - // diagnostics. Pin that non-canonical truthy-looking values ('0', 'false') - // are treated as OFF, so the skip stays silent. A truthy-gated reader would - // have emitted on these; this guards the unified strict gate (incl. the - // main() catch handler) across the claude/plugin copies. + // the stderr diagnostic. Pin that non-canonical truthy-looking values ('0', + // 'false') are treated as OFF, so stderr stays silent. The #2396 MCP-query + // hint on stdout is independent of GITNEXUS_DEBUG (it is the augmentation, not + // a diagnostic) and must still be emitted here. for (const debugValue of ['0', 'false']) { it.skipIf(SKIP_LSOF_PATH)( - `${label}: MCP-owner skip stays SILENT with GITNEXUS_DEBUG='${debugValue}' (strict contract)`, + `${label}: MCP-owner hint emits on stdout; stderr stays silent with GITNEXUS_DEBUG='${debugValue}'`, () => { const markerPath = path.join( os.tmpdir(), @@ -1933,7 +1940,8 @@ describe('PreToolUse augmentation filtering (integration)', () => { { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: debugValue } }, ); - expect(result.stdout.trim()).toBe(''); + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.stderr.trim()).toBe(''); expect(result.status).toBe(0); expect(fs.existsSync(markerPath)).toBe(false); @@ -1948,6 +1956,143 @@ describe('PreToolUse augmentation filtering (integration)', () => { } }); +// #2396: the owner-path hint is throttled to at most once per repo per window +// (GITNEXUS_MCP_HINT_THROTTLE_MS, default 10min) via a per-repo `.mcp-hint-shown` +// marker, so an owner-locked session isn't nudged on every search. macOS/other- +// Unix lsof+ps lane only (SKIP_LSOF_PATH), like the sibling owner tests. hookEnv +// sets the window to 0 (disabled) elsewhere for determinism; here we set a real +// window to exercise the throttle. +describe.skipIf(SKIP_LSOF_PATH)('MCP-owner hint throttle (#2396)', () => { + for (const [label, hookPath] of [ + ['CJS', CJS_HOOK], + ['Plugin', PLUGIN_HOOK], + ] as const) { + it(`${label}: emits once, then throttles within the window (marker gates it)`, () => { + const markerPath = path.join(os.tmpdir(), `gn-hook-throttle-${process.pid}-${label}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + const throttleMarker = path.join(gitNexusDir, '.mcp-hint-shown'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(throttleMarker, { force: true }); + const binDir = createHookToolDir({ + gitnexusMarkerPath: markerPath, + lsofOutput: '12345\n', + psOutput: 'node /tmp/node_modules/.bin/gitnexus mcp\n', + }); + const runOnce = () => + runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { env: { ...hookEnv(binDir), GITNEXUS_MCP_HINT_THROTTLE_MS: '600000' } }, + ); + try { + // First owner-locked search: emits the hint and writes the marker. + const first = runOnce(); + const out1 = parseHookOutput(first.stdout); + expect(out1!.additionalContext).toContain('mcp__gitnexus__query'); + expect(first.status).toBe(0); + expect(fs.existsSync(throttleMarker)).toBe(true); + // Second search, marker still fresh (10-min window): throttled — no hint. + const second = runOnce(); + expect(second.stdout.trim()).toBe(''); + expect(second.status).toBe(0); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(throttleMarker, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + } +}); + +// #2396: buildMcpQueryHint and its throttle are triplicated across the three hook +// copies (the repo's deliberate no-shared-module hook convention). Guard against +// silent drift with a source-level byte-identity check — runs on every platform, +// unlike the owner-path behavior tests which are macOS-only. +describe('hook copy drift guard (#2396)', () => { + const ANTIGRAVITY_HOOK = path.resolve( + __dirname, + '..', + '..', + 'hooks', + 'antigravity', + 'gitnexus-antigravity-hook.cjs', + ); + const HOOK_SOURCES: ReadonlyArray = [ + ['claude', CJS_HOOK], + ['plugin', PLUGIN_HOOK], + ['antigravity', ANTIGRAVITY_HOOK], + ]; + + function extractFn(source: string, name: string): string { + const match = source.match(new RegExp(`function ${name}\\([^)]*\\) \\{[\\s\\S]*?\\n\\}`)); + return match ? match[0] : `<${name} not found>`; + } + + for (const fnName of ['buildMcpQueryHint', 'shouldEmitMcpHint']) { + it(`${fnName} is byte-identical across all three hook copies`, () => { + const [claude, plugin, antigravity] = HOOK_SOURCES.map(([, p]) => + extractFn(fs.readFileSync(p, 'utf-8'), fnName), + ); + expect(claude).toContain(`function ${fnName}`); + expect(plugin).toBe(claude); + expect(antigravity).toBe(claude); + }); + } +}); + +// #2396: an adversarial search pattern (embedded quote + newline) must not break +// the additionalContext JSON envelope — JSON.stringify in the emit path escapes it +// structurally. Owner-path only (macOS/other-Unix lsof+ps lane, SKIP_LSOF_PATH). +describe.skipIf(SKIP_LSOF_PATH)('MCP hint pattern escaping (#2396)', () => { + for (const [label, hookPath] of [ + ['CJS', CJS_HOOK], + ['Plugin', PLUGIN_HOOK], + ] as const) { + it(`${label}: quote+newline pattern stays JSON-safe in additionalContext`, () => { + const markerPath = path.join(os.tmpdir(), `gn-hook-esc-${process.pid}-${label}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + const binDir = createHookToolDir({ + gitnexusMarkerPath: markerPath, + lsofOutput: '12345\n', + psOutput: 'node /tmp/node_modules/.bin/gitnexus mcp\n', + }); + const evilPattern = 'foo"bar\nbaz'; + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: evilPattern }, + cwd: tmpDir, + }, + undefined, + { env: hookEnv(binDir) }, + ); + // parseHookOutput JSON.parses stdout — a broken envelope would throw/return null. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('foo"bar'); + expect(output!.additionalContext).toContain('search_query'); + expect(result.status).toBe(0); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + } +}); + describe.skipIf(SKIP_LSOF_PATH)( 'Ladybug DB owner guard — production-shaped ps + failure modes (#1493)', () => { @@ -1990,7 +2135,11 @@ describe.skipIf(SKIP_LSOF_PATH)( undefined, { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); - expect(result.stdout.trim()).toBe(''); + // #2396: owner path now hands the agent the MCP-query hint on stdout; + // the CLI augment is still skipped (marker absent) and the stderr + // skip diagnostic remains debug-gated. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); @@ -2054,7 +2203,11 @@ describe.skipIf(SKIP_LSOF_PATH)( undefined, { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); - expect(result.stdout.trim()).toBe(''); + // #2396: owner path now hands the agent the MCP-query hint on stdout; + // the CLI augment is still skipped (marker absent) and the stderr + // skip diagnostic remains debug-gated. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); @@ -2142,7 +2295,11 @@ describe.skipIf(SKIP_LSOF_PATH)( undefined, { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); - expect(result.stdout.trim()).toBe(''); + // #2396: owner path now hands the agent the MCP-query hint on stdout; + // the CLI augment is still skipped (marker absent) and the stderr + // skip diagnostic remains debug-gated. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); @@ -2152,11 +2309,13 @@ describe.skipIf(SKIP_LSOF_PATH)( } }); - // #1913: the fail-closed (probe-timeout) skip routes through the SAME gated - // line as the MCP-owner skip, so it too must be silent by default. Symmetric - // counterpart to the debug-on test above, so a regression that ungated the - // ETIMEDOUT path specifically would still be caught. - it(`${label}: ETIMEDOUT lsof → augment skipped SILENTLY by default`, () => { + // #2396/#1913: the fail-closed (probe-timeout) skip routes through the SAME + // owner branch, so it now emits the conditional MCP-query hint on stdout — + // truthful here because the hint only asks the agent to use the MCP tools + // "if they are live". The stderr diagnostic stays debug-gated (empty by + // default), so strict runners still see no unexpected diagnostic. Symmetric + // counterpart to the debug-on test above. + it(`${label}: ETIMEDOUT lsof → emits hint on stdout, stderr silent by default`, () => { const markerPath = path.join(os.tmpdir(), `gn-hook-etime-silent-${process.pid}-${label}`); const lbugPath = path.join(gitNexusDir, 'lbug'); fs.writeFileSync(lbugPath, ''); @@ -2178,7 +2337,8 @@ describe.skipIf(SKIP_LSOF_PATH)( undefined, { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '' } }, ); - expect(result.stdout.trim()).toBe(''); + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.stderr.trim()).toBe(''); expect(result.status).toBe(0); expect(fs.existsSync(markerPath)).toBe(false); @@ -2226,7 +2386,11 @@ describe.skipIf(SKIP_LSOF_PATH)( }, }, ); - expect(result.stdout.trim()).toBe(''); + // #2396: owner path now hands the agent the MCP-query hint on stdout; + // the CLI augment is still skipped (marker absent) and the stderr + // skip diagnostic remains debug-gated. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); @@ -2279,7 +2443,11 @@ describe.skipIf(SKIP_LSOF_PATH)( }, }, ); - expect(result.stdout.trim()).toBe(''); + // #2396: owner path now hands the agent the MCP-query hint on stdout; + // the CLI augment is still skipped (marker absent) and the stderr + // skip diagnostic remains debug-gated. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); @@ -2335,7 +2503,11 @@ describe.skipIf(SKIP_LSOF_PATH)( }, }, ); - expect(result.stdout.trim()).toBe(''); + // #2396: owner path now hands the agent the MCP-query hint on stdout; + // the CLI augment is still skipped (marker absent) and the stderr + // skip diagnostic remains debug-gated. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); @@ -2391,7 +2563,11 @@ describe.skipIf(SKIP_LSOF_PATH)( }, }, ); - expect(result.stdout.trim()).toBe(''); + // #2396: owner path now hands the agent the MCP-query hint on stdout; + // the CLI augment is still skipped (marker absent) and the stderr + // skip diagnostic remains debug-gated. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); @@ -2516,7 +2692,11 @@ describe.skipIf(SKIP_LSOF_PATH)( undefined, { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); - expect(result.stdout.trim()).toBe(''); + // #2396: owner path now hands the agent the MCP-query hint on stdout; + // the CLI augment is still skipped (marker absent) and the stderr + // skip diagnostic remains debug-gated. + const output = parseHookOutput(result.stdout); + expect(output!.additionalContext).toContain('mcp__gitnexus__query'); expect(result.status).toBe(0); expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); diff --git a/gitnexus/test/utils/hook-test-helpers.ts b/gitnexus/test/utils/hook-test-helpers.ts index f5203a023..a7838971a 100644 --- a/gitnexus/test/utils/hook-test-helpers.ts +++ b/gitnexus/test/utils/hook-test-helpers.ts @@ -220,6 +220,10 @@ export function hookEnv(binDir: string) { GITNEXUS_HOOK_CLI_PATH: path.join(binDir, 'gitnexus-cli.js'), GITNEXUS_HOOK_LSOF_PATH: path.join(binDir, 'lsof'), GITNEXUS_HOOK_PS_PATH: path.join(binDir, 'ps'), + // #2396: disable the per-repo MCP-hint throttle by default so owner tests + // are deterministic (gitNexusDir is shared across the suite). The throttle + // itself is covered by its own dedicated test, which sets a real window. + GITNEXUS_MCP_HINT_THROTTLE_MS: '0', }; }