diff --git a/gitnexus-claude-plugin/hooks/gitnexus-hook.js b/gitnexus-claude-plugin/hooks/gitnexus-hook.js index e3c62c769..c3ec2ecf5 100644 --- a/gitnexus-claude-plugin/hooks/gitnexus-hook.js +++ b/gitnexus-claude-plugin/hooks/gitnexus-hook.js @@ -110,10 +110,20 @@ function hasGitNexusServerOwner(gitNexusDir) { return hasGitNexusDbLockedByGitNexusServer(path.join(gitNexusDir, 'lbug'), process.pid); } +/** + * Whether opt-in diagnostics should be written to the hook's stderr. Strict + * hook runners (e.g. Codex `PreToolUse`) validate hook output, so normal, + * non-error skip paths must stay silent unless the operator explicitly asks + * for diagnostics via GITNEXUS_DEBUG. See issue #1913. + */ +function isDebugEnabled() { + return process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true'; +} + function extractAugmentContext(stderr) { const output = (stderr || '').trim(); const marker = output.indexOf('[GitNexus]'); - const debug = process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true'; + const debug = isDebugEnabled(); if (debug && output.length > 0) { // Emit the FULL discarded prefix (everything before the marker, or all of // it when no marker is present) so suppressed diagnostics — LadybugDB lock @@ -267,7 +277,12 @@ function handlePreToolUse(input) { const pattern = extractPattern(toolName, toolInput); if (!pattern || pattern.length < 3) return; if (hasGitNexusServerOwner(gitNexusDir)) { - process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + // 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. + if (isDebugEnabled()) { + process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + } return; } @@ -366,7 +381,7 @@ function main() { const handler = handlers[input.hook_event_name || '']; if (handler) handler(input); } catch (err) { - if (process.env.GITNEXUS_DEBUG) { + if (isDebugEnabled()) { console.error('GitNexus hook error:', (err.message || '').slice(0, 200)); } } diff --git a/gitnexus/README.md b/gitnexus/README.md index d6ea20641..4f5b27781 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -436,6 +436,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 + +The Claude Code / Antigravity hooks intentionally stay **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. + +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: + +```bash +GITNEXUS_DEBUG=1 # surfaces hook skip/diagnostic reasons on stderr +``` + +Only `GITNEXUS_DEBUG=1` and `GITNEXUS_DEBUG=true` enable diagnostics; every other +value (including `0` and `false`) is treated as off. Diagnostics go to stderr +only — the hook's structured stdout (the JSON the agent consumes) is unaffected. + ## Privacy - All processing happens locally on your machine diff --git a/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs b/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs index bbfccb92e..0d837fb2c 100755 --- a/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs +++ b/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs @@ -91,10 +91,20 @@ function hasGitNexusServerOwner(gitNexusDir) { return hasGitNexusDbLockedByGitNexusServer(path.join(gitNexusDir, 'lbug'), process.pid); } +/** + * Whether opt-in diagnostics should be written to the hook's stderr. Strict + * hook runners validate hook output, so normal, non-error skip paths must stay + * silent unless the operator explicitly asks for diagnostics via GITNEXUS_DEBUG. + * See issue #1913. + */ +function isDebugEnabled() { + return process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true'; +} + function extractAugmentContext(stderr) { const output = (stderr || '').trim(); const marker = output.indexOf('[GitNexus]'); - const debug = process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true'; + const debug = isDebugEnabled(); if (debug && output.length > 0) { // Emit the FULL discarded prefix (everything before the marker, or all of // it when no marker is present) so suppressed diagnostics — LadybugDB lock @@ -258,8 +268,14 @@ function buildAfterToolContext(input) { if (/\bgit\s+(commit|merge|rebase|cherry-pick|pull)(\s|$)/.test(command)) { const hint = buildStaleIndexHint(gitNexusDir, cwd); if (hint) { - process.stderr.write(`${hint}\n`); + // The hint always reaches the agent via additionalContext (parts). Mirror + // it to stderr (for terminal users) only under GITNEXUS_DEBUG, so strict + // hook runners see no unexpected output on this normal path (#1913). The + // claude hook never mirrored this to stderr — this aligns the two adapters. parts.push(hint); + if (isDebugEnabled()) { + process.stderr.write(`${hint}\n`); + } } } } @@ -269,7 +285,11 @@ function buildAfterToolContext(input) { function runAugment(gitNexusDir, cwd, pattern) { if (hasGitNexusServerOwner(gitNexusDir)) { - process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + // Normal skip path: the MCP server owns the DB. Stay silent for strict + // hook runners (issue #1913); surface the reason only under GITNEXUS_DEBUG. + if (isDebugEnabled()) { + process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + } return ''; } const release = acquireHookSlot(gitNexusDir); @@ -338,7 +358,7 @@ function main() { const handler = handlers[input.hook_event_name || '']; if (handler) handler(input); } catch (err) { - if (process.env.GITNEXUS_DEBUG) { + if (isDebugEnabled()) { console.error('GitNexus antigravity hook error:', (err.message || '').slice(0, 200)); } } diff --git a/gitnexus/hooks/claude/gitnexus-hook.cjs b/gitnexus/hooks/claude/gitnexus-hook.cjs index 8bfa49381..40d0b08df 100755 --- a/gitnexus/hooks/claude/gitnexus-hook.cjs +++ b/gitnexus/hooks/claude/gitnexus-hook.cjs @@ -110,10 +110,20 @@ function hasGitNexusServerOwner(gitNexusDir) { return hasGitNexusDbLockedByGitNexusServer(path.join(gitNexusDir, 'lbug'), process.pid); } +/** + * Whether opt-in diagnostics should be written to the hook's stderr. Strict + * hook runners (e.g. Codex `PreToolUse`) validate hook output, so normal, + * non-error skip paths must stay silent unless the operator explicitly asks + * for diagnostics via GITNEXUS_DEBUG. See issue #1913. + */ +function isDebugEnabled() { + return process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true'; +} + function extractAugmentContext(stderr) { const output = (stderr || '').trim(); const marker = output.indexOf('[GitNexus]'); - const debug = process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true'; + const debug = isDebugEnabled(); if (debug && output.length > 0) { // Emit the FULL discarded prefix (everything before the marker, or all of // it when no marker is present) so suppressed diagnostics — KuzuDB lock @@ -250,7 +260,12 @@ function handlePreToolUse(input) { const pattern = extractPattern(toolName, toolInput); if (!pattern || pattern.length < 3) return; if (hasGitNexusServerOwner(gitNexusDir)) { - process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + // 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. + if (isDebugEnabled()) { + process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + } return; } @@ -361,7 +376,7 @@ function main() { const handler = handlers[input.hook_event_name || '']; if (handler) handler(input); } catch (err) { - if (process.env.GITNEXUS_DEBUG) { + if (isDebugEnabled()) { console.error('GitNexus hook error:', (err.message || '').slice(0, 200)); } } diff --git a/gitnexus/test/integration/antigravity-hook-e2e.test.ts b/gitnexus/test/integration/antigravity-hook-e2e.test.ts index 33b2ca56f..a4a9d1f01 100644 --- a/gitnexus/test/integration/antigravity-hook-e2e.test.ts +++ b/gitnexus/test/integration/antigravity-hook-e2e.test.ts @@ -26,6 +26,8 @@ import { runHook, parseHookOutput, createGitNexusPathEntry, + createHookToolDir, + hookEnv, envWithPath, } from '../utils/hook-test-helpers.js'; import { setupCommand } from '../../src/cli/setup.js'; @@ -101,7 +103,10 @@ afterAll(async () => { describe('antigravity hook adapter e2e', () => { describe('AfterTool — stale-index hint after git mutations', () => { - it('emits the hint via both additionalContext and stderr after a successful git commit', () => { + // #1913: by default the hint reaches the agent via additionalContext (stdout + // JSON) but is NOT mirrored to stderr, so strict hook runners see no + // unexpected output on this normal (non-error) path. + it('emits the hint via additionalContext and stays silent on stderr by default', () => { fs.writeFileSync( path.join(gitNexusDir, 'meta.json'), JSON.stringify({ lastCommit: 'a'.repeat(40), stats: {} }), @@ -117,7 +122,7 @@ describe('antigravity hook adapter e2e', () => { cwd: tmpDir, }, tmpDir, - { env: { ...process.env, GITNEXUS_INVOCATION: 'npx' } }, + { env: { ...process.env, GITNEXUS_INVOCATION: 'npx', GITNEXUS_DEBUG: '' } }, ); const output = parseHookOutput(result.stdout); @@ -125,9 +130,33 @@ describe('antigravity hook adapter e2e', () => { expect(output!.hookEventName).toBe('AfterTool'); expect(output!.additionalContext).toContain('index is stale'); expect(output!.additionalContext).toContain('npx gitnexus@latest analyze'); + // Strict-runner contract: the hint is NOT mirrored to stderr by default. + expect(result.stderr).not.toContain('[GitNexus] index is stale'); + }); - // Mirror to stderr so terminal users see the hint even when the agent - // discards additionalContext + // #1913: the terminal-mirror remains available for operators who opt in. + it('mirrors the hint to stderr for terminal users only under GITNEXUS_DEBUG=1', () => { + fs.writeFileSync( + path.join(gitNexusDir, 'meta.json'), + JSON.stringify({ lastCommit: 'a'.repeat(40), stats: {} }), + ); + + const result = runHook( + installedHook, + { + hook_event_name: 'AfterTool', + tool_name: 'run_shell_command', + tool_input: { command: 'git commit -m "test"' }, + tool_response: { llmContent: '[committed]' }, + cwd: tmpDir, + }, + tmpDir, + { env: { ...process.env, GITNEXUS_INVOCATION: 'npx', GITNEXUS_DEBUG: '1' } }, + ); + + const output = parseHookOutput(result.stdout); + expect(output).not.toBeNull(); + expect(output!.additionalContext).toContain('index is stale'); expect(result.stderr).toContain('[GitNexus] index is stale'); }); @@ -359,6 +388,91 @@ 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. + describe.skipIf(process.platform === 'win32')( + 'AfterTool — augment skipped when MCP server owns the DB (#1913)', + () => { + 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)', () => { + const markerPath = path.join(os.tmpdir(), `antigravity-skip-silent-${process.pid}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + const binDir = createHookToolDir({ ...OWNER_PROBE, gitnexusMarkerPath: markerPath }); + try { + const result = runHook( + installedHook, + { + hook_event_name: 'AfterTool', + tool_name: 'search_file_content', + tool_input: { pattern: 'validateUser' }, + tool_response: { llmContent: '...' }, + cwd: tmpDir, + }, + tmpDir, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '' } }, + ); + + 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(''); + 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). + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + + it('surfaces the skip reason on stderr only under GITNEXUS_DEBUG=1', () => { + const markerPath = path.join(os.tmpdir(), `antigravity-skip-debug-${process.pid}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + const binDir = createHookToolDir({ ...OWNER_PROBE, gitnexusMarkerPath: markerPath }); + try { + const result = runHook( + installedHook, + { + hook_event_name: 'AfterTool', + tool_name: 'search_file_content', + tool_input: { pattern: 'validateUser' }, + tool_response: { llmContent: '...' }, + cwd: tmpDir, + }, + tmpDir, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, + ); + + expect(result.status).toBe(0); + expect(parseHookOutput(result.stdout)).toBeNull(); + expect(result.stderr).toContain('[GitNexus] augment skipped: MCP server owns DB'); + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + }, + ); + describe('cwd validation', () => { it('rejects relative cwd silently', () => { const result = runHook(installedHook, { diff --git a/gitnexus/test/unit/hooks.test.ts b/gitnexus/test/unit/hooks.test.ts index ef8f9afef..54193d837 100644 --- a/gitnexus/test/unit/hooks.test.ts +++ b/gitnexus/test/unit/hooks.test.ts @@ -22,7 +22,12 @@ import { spawnSync } from 'child_process'; import fs from 'fs'; import path from 'path'; import os from 'os'; -import { runHook, parseHookOutput } from '../utils/hook-test-helpers.js'; +import { + runHook, + parseHookOutput, + createHookToolDir, + hookEnv, +} from '../utils/hook-test-helpers.js'; // ─── Paths to both hook variants ──────────────────────────────────── @@ -145,61 +150,8 @@ function createGlobalRegistry(homeDir: string, marker: 'both' | 'registry' | 're } } -function writeExecutable(filePath: string, content: string) { - fs.writeFileSync(filePath, content, { mode: 0o755 }); -} - -function createHookToolDir(options: { - gitnexusStderr?: string; - gitnexusMarkerPath?: string; - lsofOutput?: string; - lsofOutputLines?: string[]; - psOutput?: string; - psOutputByPid?: Record; - lsofSleepMs?: number; -}) { - const binDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-hook-bin-')); - const gitnexusStderr = JSON.stringify(options.gitnexusStderr ?? ''); - const markerPath = JSON.stringify(options.gitnexusMarkerPath ?? ''); - - const fakeGitNexus = `#!/usr/bin/env node\nconst fs = require('fs');\nconst marker = ${markerPath};\nif (marker) fs.writeFileSync(marker, 'called');\nprocess.stderr.write(${gitnexusStderr});\n`; - writeExecutable(path.join(binDir, 'gitnexus'), fakeGitNexus); - writeExecutable(path.join(binDir, 'gitnexus-cli.js'), fakeGitNexus); - - const lsofOutput = - options.lsofOutputLines != null - ? options.lsofOutputLines.join('\n') + (options.lsofOutputLines.length ? '\n' : '') - : (options.lsofOutput ?? ''); - const lsofBody = - options.lsofSleepMs != null - ? `#!/usr/bin/env node\nsetTimeout(() => {}, ${Number(options.lsofSleepMs)});\n` - : `#!/usr/bin/env node\nprocess.stdout.write(${JSON.stringify(lsofOutput)});\nprocess.exit(0);\n`; - writeExecutable(path.join(binDir, 'lsof'), lsofBody); - - const psBody = - options.psOutputByPid != null - ? `#!/usr/bin/env node -const byPid = ${JSON.stringify(options.psOutputByPid)}; -const args = process.argv; -const p = args[args.indexOf('-p') + 1]; -process.stdout.write(byPid[p] ?? ''); -process.exit(0); -` - : `#!/usr/bin/env node\nprocess.stdout.write(${JSON.stringify(options.psOutput ?? '')});\nprocess.exit(0);\n`; - writeExecutable(path.join(binDir, 'ps'), psBody); - - return binDir; -} - -function hookEnv(binDir: string) { - return { - ...process.env, - PATH: `${binDir}${path.delimiter}${process.env.PATH || ''}`, - 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'), - }; -} +// createHookToolDir / hookEnv live in ../utils/hook-test-helpers so the antigravity +// e2e suite can reuse the same DB-owner-probe fakes. // ─── Both hook files should exist ─────────────────────────────────── @@ -972,8 +924,13 @@ 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. it.skipIf(process.platform === 'win32')( - `${label}: skips augment when a GitNexus MCP process owns the repo DB`, + `${label}: skips augment SILENTLY 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'); @@ -994,25 +951,118 @@ describe('PreToolUse augmentation filtering (integration)', () => { cwd: tmpDir, }, undefined, - { env: hookEnv(binDir) }, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '' } }, ); expect(result.stdout.trim()).toBe(''); + expect(result.stderr.trim()).toBe(''); expect(result.status).toBe(0); - expect(result.stderr).toContain('[GitNexus] augment skipped'); expect(fs.existsSync(markerPath)).toBe(false); } finally { + fs.rmSync(lbugPath, { force: true }); fs.rmSync(markerPath, { force: true }); fs.rmSync(binDir, { recursive: true, force: true }); } }, ); + + // 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. + it.skipIf(process.platform === 'win32')( + `${label}: surfaces the MCP-owner skip reason only under GITNEXUS_DEBUG`, + () => { + const markerPath = path.join(os.tmpdir(), `gitnexus-hook-dbg-${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', + }); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, + ); + + expect(result.stdout.trim()).toBe(''); + expect(result.status).toBe(0); + expect(result.stderr).toContain('[GitNexus] augment skipped: MCP server owns DB'); + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }, + ); + + // #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. + for (const debugValue of ['0', 'false']) { + it.skipIf(process.platform === 'win32')( + `${label}: MCP-owner skip stays SILENT with GITNEXUS_DEBUG='${debugValue}' (strict contract)`, + () => { + const markerPath = path.join( + os.tmpdir(), + `gitnexus-hook-dbg-${debugValue}-${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', + }); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: debugValue } }, + ); + + expect(result.stdout.trim()).toBe(''); + expect(result.stderr.trim()).toBe(''); + expect(result.status).toBe(0); + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }, + ); + } } }); describe.skipIf(process.platform === 'win32')( 'Ladybug DB owner guard — production-shaped ps + failure modes (#1493)', () => { + // These tests assert owner *detection*: a positive skip is signalled by the + // `[GitNexus] augment skipped` diagnostic. Since #1913 made that diagnostic + // debug-gated (silent by default for strict hook runners), they run with + // GITNEXUS_DEBUG=1 so the discriminator remains observable. Default-silence + // itself is covered by the 'augmentation filtering' describe above. for (const [label, hookPath] of [ ['CJS', CJS_HOOK], ['Plugin', PLUGIN_HOOK], @@ -1037,7 +1087,7 @@ describe.skipIf(process.platform === 'win32')( cwd: tmpDir, }, undefined, - { env: hookEnv(binDir) }, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); expect(result.stdout.trim()).toBe(''); expect(result.status).toBe(0); @@ -1101,7 +1151,7 @@ describe.skipIf(process.platform === 'win32')( cwd: tmpDir, }, undefined, - { env: hookEnv(binDir) }, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); expect(result.stdout.trim()).toBe(''); expect(result.status).toBe(0); @@ -1169,7 +1219,7 @@ describe.skipIf(process.platform === 'win32')( cwd: tmpDir, }, undefined, - { env: hookEnv(binDir) }, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); expect(result.stdout.trim()).toBe(''); expect(result.status).toBe(0); @@ -1181,6 +1231,43 @@ describe.skipIf(process.platform === 'win32')( } }); + // #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`, () => { + const markerPath = path.join(os.tmpdir(), `gn-hook-etime-silent-${process.pid}-${label}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + const binDir = createHookToolDir({ + gitnexusMarkerPath: markerPath, + lsofSleepMs: 5000, + psOutput: '', + }); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '' } }, + ); + expect(result.stdout.trim()).toBe(''); + expect(result.stderr.trim()).toBe(''); + expect(result.status).toBe(0); + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + it(`${label}: non-GitNexus ps line → augment runs`, () => { const markerPath = path.join(os.tmpdir(), `gn-hook-other-${process.pid}-${label}`); const lbugPath = path.join(gitNexusDir, 'lbug'); @@ -1237,7 +1324,7 @@ describe.skipIf(process.platform === 'win32')( cwd: tmpDir, }, undefined, - { env: hookEnv(binDir) }, + { env: { ...hookEnv(binDir), GITNEXUS_DEBUG: '1' } }, ); expect(result.stdout.trim()).toBe(''); expect(result.status).toBe(0); diff --git a/gitnexus/test/unit/setup-antigravity.test.ts b/gitnexus/test/unit/setup-antigravity.test.ts index 73a1cb305..42c58bb47 100644 --- a/gitnexus/test/unit/setup-antigravity.test.ts +++ b/gitnexus/test/unit/setup-antigravity.test.ts @@ -428,29 +428,36 @@ describe('gitnexus-antigravity-hook adapter', () => { 'utf-8', ); - const { stdout, stderr } = runAdapter( - adapter, - { - hook_event_name: 'AfterTool', - tool_name: 'run_shell_command', - tool_input: { command: 'git commit -m "x"' }, - tool_response: { llmContent: '[committed]' }, - cwd: workdir, - }, - workdir, - // Force a deterministic invocation mode: the emitted analyze command - // varies by what's installed on each CI runner (gitnexus/pnpm/npx), and - // only the `gitnexus` mode yields the bare `gitnexus analyze` form. - { GITNEXUS_INVOCATION: 'gitnexus' }, - ); - - // Hint surfaces both via the agent-visible channel and stderr (terminal). - expect(stderr).toMatch(/\[GitNexus\] index is stale/); - expect(stderr).toMatch(/gitnexus analyze/); + const input = { + hook_event_name: 'AfterTool', + tool_name: 'run_shell_command', + tool_input: { command: 'git commit -m "x"' }, + tool_response: { llmContent: '[committed]' }, + cwd: workdir, + }; + // Force a deterministic invocation mode: the emitted analyze command varies + // by what's installed on each CI runner (gitnexus/pnpm/npx); only the + // `gitnexus` mode yields the bare `gitnexus analyze` form. + const { stdout, stderr } = runAdapter(adapter, input, workdir, { + GITNEXUS_INVOCATION: 'gitnexus', + GITNEXUS_DEBUG: '', + }); + // #1913: by default the hint reaches the agent via additionalContext (stdout + // JSON) but is NOT mirrored to stderr, so strict hook runners stay clean. const parsed = JSON.parse(stdout); expect(parsed.hookSpecificOutput.hookEventName).toBe('AfterTool'); expect(parsed.hookSpecificOutput.additionalContext).toMatch(/index is stale/); + expect(parsed.hookSpecificOutput.additionalContext).toMatch(/gitnexus analyze/); + expect(stderr).not.toMatch(/\[GitNexus\] index is stale/); + + // The terminal mirror remains available under GITNEXUS_DEBUG=1. + const debug = runAdapter(adapter, input, workdir, { + GITNEXUS_INVOCATION: 'gitnexus', + GITNEXUS_DEBUG: '1', + }); + expect(debug.stderr).toMatch(/\[GitNexus\] index is stale/); + expect(debug.stderr).toMatch(/gitnexus analyze/); }); it('AfterTool skips augment when the tool failed', async () => { diff --git a/gitnexus/test/utils/hook-test-helpers.ts b/gitnexus/test/utils/hook-test-helpers.ts index f659a5834..1baa94ea6 100644 --- a/gitnexus/test/utils/hook-test-helpers.ts +++ b/gitnexus/test/utils/hook-test-helpers.ts @@ -72,6 +72,71 @@ function hasGitNexusLauncher(dir: string): boolean { }); } +// ─── Fake tool dir for the DB-owner probe (shared by unit + e2e) ──── +// +// Builds a temp bin dir holding fake `gitnexus`, `lsof`, and `ps` executables so +// a hook spawned with hookEnv(binDir) sees a deterministic DB-owner probe result +// (and a marker-writing fake CLI) without touching the real process table. + +// Module-private: only createHookToolDir writes these fakes; callers use the +// higher-level createHookToolDir, never writeExecutable directly. +function writeExecutable(filePath: string, content: string) { + fs.writeFileSync(filePath, content, { mode: 0o755 }); +} + +export function createHookToolDir(options: { + gitnexusStderr?: string; + gitnexusMarkerPath?: string; + lsofOutput?: string; + lsofOutputLines?: string[]; + psOutput?: string; + psOutputByPid?: Record; + lsofSleepMs?: number; +}) { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-hook-bin-')); + const gitnexusStderr = JSON.stringify(options.gitnexusStderr ?? ''); + const markerPath = JSON.stringify(options.gitnexusMarkerPath ?? ''); + + const fakeGitNexus = `#!/usr/bin/env node\nconst fs = require('fs');\nconst marker = ${markerPath};\nif (marker) fs.writeFileSync(marker, 'called');\nprocess.stderr.write(${gitnexusStderr});\n`; + writeExecutable(path.join(binDir, 'gitnexus'), fakeGitNexus); + writeExecutable(path.join(binDir, 'gitnexus-cli.js'), fakeGitNexus); + + const lsofOutput = + options.lsofOutputLines != null + ? options.lsofOutputLines.join('\n') + (options.lsofOutputLines.length ? '\n' : '') + : (options.lsofOutput ?? ''); + const lsofBody = + options.lsofSleepMs != null + ? `#!/usr/bin/env node\nsetTimeout(() => {}, ${Number(options.lsofSleepMs)});\n` + : `#!/usr/bin/env node\nprocess.stdout.write(${JSON.stringify(lsofOutput)});\nprocess.exit(0);\n`; + writeExecutable(path.join(binDir, 'lsof'), lsofBody); + + const psBody = + options.psOutputByPid != null + ? `#!/usr/bin/env node +const byPid = ${JSON.stringify(options.psOutputByPid)}; +const args = process.argv; +const p = args[args.indexOf('-p') + 1]; +process.stdout.write(byPid[p] ?? ''); +process.exit(0); +` + : `#!/usr/bin/env node\nprocess.stdout.write(${JSON.stringify(options.psOutput ?? '')});\nprocess.exit(0);\n`; + writeExecutable(path.join(binDir, 'ps'), psBody); + + return binDir; +} + +/** A full env that points a spawned hook at the fake tool dir from createHookToolDir. */ +export function hookEnv(binDir: string) { + return { + ...process.env, + PATH: `${binDir}${path.delimiter}${process.env.PATH || ''}`, + 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'), + }; +} + /** * The current PATH with every dir that contains a `gitnexus` launcher removed, so * a test box that already has gitnexus installed cannot make the assertion pass