diff --git a/Dockerfile.cli b/Dockerfile.cli index b5ec3390c..633d23f5d 100644 --- a/Dockerfile.cli +++ b/Dockerfile.cli @@ -100,6 +100,22 @@ ENV HOME=/home/node \ RUN su node -s /bin/sh -c "HOME=/home/node node /app/gitnexus/scripts/install-duckdb-extension.mjs fts" \ && su node -s /bin/sh -c "HOME=/home/node node /app/gitnexus/scripts/install-duckdb-extension.mjs fts --verify-only" +# Published runtime assets (in package.json `files`). Placed AFTER the DuckDB +# FTS-extension RUN above so editing hook/skill content does not invalidate that +# network-fetching cache layer; they have no input dependency on it. +# `hooks/`: dist/cli/resolve-invocation.js does +# `require('../../hooks/claude/resolve-analyze-cmd.cjs')` at module load — the +# single source of truth for the npm-11 npx-crash invocation decision (#1939). +# Without it, `gitnexus analyze` inside the image crashes with MODULE_NOT_FOUND +# before it does any work (#2130). `skills/`: the CLI reads the bundled SKILL.md +# templates from `/skills/` for `gitnexus analyze --skills` and `gitnexus +# setup`/`uninstall`; absent, those degrade silently (placeholder content / zero +# skills installed). (The web UI bundle `web/`, also in `files`, is deliberately +# NOT shipped: this builder never builds gitnexus-web, so the image is API-only; +# the UI is the separate Dockerfile.web image / hosted app.) +COPY --from=builder --chown=node:node /app/gitnexus/hooks ./gitnexus/hooks +COPY --from=builder --chown=node:node /app/gitnexus/skills ./gitnexus/skills + USER node # The web UI defaults to http://localhost:4747 - keep that contract. 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/dockerfile-runtime-asset-parity.test.ts b/gitnexus/test/unit/dockerfile-runtime-asset-parity.test.ts new file mode 100644 index 000000000..226b57d3f --- /dev/null +++ b/gitnexus/test/unit/dockerfile-runtime-asset-parity.test.ts @@ -0,0 +1,445 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync, readdirSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** + * Regression guard for #2130. + * + * `Dockerfile.cli`'s runtime stage hand-copies a SUBSET of the package's + * published assets (`package.json` `files` = dist, hooks, scripts, skills, + * vendor, web) out of the builder. npm ships all of `files`, but the Docker + * image copies only what it thinks it needs — so when compiled `dist/**` gains a + * `require()`/`createRequire()` into a sibling directory that the runtime stage + * does NOT copy, the image crashes with `MODULE_NOT_FOUND` at module load while + * the npm package keeps working. That is exactly #2130: `dist/cli/ + * resolve-invocation.js` does `require('../../hooks/claude/resolve-analyze-cmd.cjs')` + * (statically imported by `analyze.ts`), but the runtime stage never copied + * `hooks/`, so `gitnexus analyze` inside the image died before doing any work. + * + * This test derives, from the SOURCE tree, every out-of-`dist` asset that + * compiled code `require()`s AT MODULE LOAD, then asserts each one is covered by + * a runtime-stage `COPY --from=builder`. It is deliberately scoped to + * `require`/`createRequire` (hard module resolution — a missing target throws): + * an asset reached only via `fs.access`/`fs.readFile`/`new URL(...)` (e.g. `web/`) + * degrades gracefully when absent and is intentionally not copied, so it is out of + * scope here. `skills/` is also fs-accessed but IS shipped (covered by its own + * `it('copies skills/…')` below), because the CLI must stay fully usable. + */ + +const UNIT_DIR = path.dirname(fileURLToPath(import.meta.url)); +const GITNEXUS_ROOT = path.resolve(UNIT_DIR, '..', '..'); +const REPO_ROOT = path.resolve(GITNEXUS_ROOT, '..'); +const SRC_DIR = path.join(GITNEXUS_ROOT, 'src'); +const DOCKERFILE = path.join(REPO_ROOT, 'Dockerfile.cli'); + +const toPosix = (p: string): string => p.split(path.sep).join('/'); + +/** + * Source paths (relative to the gitnexus package root) copied into the image by + * the RUNTIME stage of Dockerfile.cli — e.g. `hooks`, + * `scripts/install-duckdb-extension.mjs`. The builder stage's full-tree + * `COPY gitnexus ./gitnexus` is ignored on purpose: it would mask every gap. + */ +function runtimeStageCopiedSources(dockerfile: string): string[] { + const lines = dockerfile.split('\n'); + // `i` flag: Docker accepts lowercase `as`, so a future reformat to + // `FROM … as runtime` must not silently lose the stage (which would empty the + // copied set and trip the named assertions below). + const runtimeStart = lines.findIndex((l) => /^FROM\s.*\bAS\s+runtime\b/i.test(l)); + expect(runtimeStart, 'Dockerfile.cli must declare a `... AS runtime` stage').toBeGreaterThan(-1); + const sources: string[] = []; + // Scan only the runtime stage: start after its FROM and stop at the next + // stage boundary, so COPY lines from any stage added AFTER runtime are never + // misattributed to it. + for (const line of lines.slice(runtimeStart + 1)) { + if (/^FROM\b/.test(line)) break; + if (!/^COPY\s+--from=builder\b/.test(line)) continue; + // The source operand is the `/app/gitnexus/` token (the dest is + // `./gitnexus/`). There is exactly one per COPY line here. + const m = line.match(/\s\/app\/gitnexus\/(\S+)/); + if (m) sources.push(m[1]); + } + return sources; +} + +const isCovered = (assetPath: string, copied: string[]): boolean => + copied.some((c) => assetPath === c || assetPath.startsWith(c + '/')); + +/** Recursively list non-test `.ts` files under a directory. */ +function listSourceFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === '__tests__' || entry.name === '__mocks__') continue; + out.push(...listSourceFiles(full)); + } else if ( + entry.name.endsWith('.ts') && + !entry.name.endsWith('.test.ts') && + !entry.name.endsWith('.spec.ts') && + !entry.name.endsWith('.d.ts') + ) { + out.push(full); + } + } + return out; +} + +// The `createRequire(...)('../x')` IIFE form (e.g. resolve-invocation.ts). +// Captures the relative specifier (starting with '.'). The built-in +// `require`/`_require` and aliased-binding literal forms are matched separately +// in `scanContent` from the file's discovered require-family identifiers, and +// computed (non-literal) module-load requires are handled there too. Dynamic/ +// static ESM `import` is excluded — TS keeps those inside `dist/`. +const CREATE_REQUIRE_IIFE_RE = /createRequire\([\s\S]*?\)\s*\(\s*['"](\.[^'"]+)['"]\s*\)/g; + +/** + * Strip `//` line comments and block comments so commented-out or documented + * requires (e.g. a JSDoc `require(computedPath)`, or a `// require('../../web/x')`) + * cannot spuriously trip the parity guard — a false-fail for the literal scan + * and a false "unverifiable computed require" for the broadened scan below. + * + * This is a small string-aware pass rather than a naive regex strip: it tracks + * `'`/`"`/`` ` `` string state so a slash-star or `//` INSIDE a string or glob + * literal (e.g. a `node_modules/` glob, a `thrift::x/` template) is never + * mistaken for a comment delimiter and used to mangle real code. Newlines are + * preserved so brace-depth accounting stays meaningful. (Verified: the real-tree + * literal-scan output is byte-identical with and without this pass.) + */ +function stripComments(content: string): string { + let out = ''; + type State = 'code' | 'line' | 'block' | 'sq' | 'dq' | 'tpl'; + let state: State = 'code'; + for (let i = 0; i < content.length; i += 1) { + const c = content[i]; + const c2 = content[i + 1]; + if (state === 'code') { + if (c === '/' && c2 === '/') { + state = 'line'; + i += 1; + } else if (c === '/' && c2 === '*') { + state = 'block'; + i += 1; + } else if (c === "'") { + state = 'sq'; + out += c; + } else if (c === '"') { + state = 'dq'; + out += c; + } else if (c === '`') { + state = 'tpl'; + out += c; + } else { + out += c; + } + } else if (state === 'line') { + if (c === '\n') { + state = 'code'; + out += c; + } + } else if (state === 'block') { + if (c === '*' && c2 === '/') { + state = 'code'; + i += 1; + } else if (c === '\n') { + out += c; + } + } else { + // inside a string/template literal — copy verbatim, honoring escapes + out += c; + if (c === '\\' && i + 1 < content.length) { + out += content[i + 1]; + i += 1; + } else if ( + (state === 'sq' && c === "'") || + (state === 'dq' && c === '"') || + (state === 'tpl' && c === '`') + ) { + state = 'code'; + } + } + } + return out; +} + +/** + * Vetted module-load requires whose target is a COMPUTED (non-literal) path the + * scanner cannot resolve statically. Maps a source file (relative to `src/`) to + * the package-relative asset it loads at module load. The asset is still run + * through the COPY-coverage check like any literal — this allowlist only + * suppresses the "unverifiable computed require" hard-fail; it never exempts the + * asset from `isCovered` (so deleting the covering COPY still fails the guard, + * enforced by the coverage-not-trust test below). + */ +const KNOWN_COMPUTED_REQUIRES: { source: string; asset: string }[] = [ + // community-processor.ts: `const leidenPath = resolve(__dirname,'..','..','..', + // 'vendor','leiden','index.cjs'); const leiden = _require(leidenPath);` + { source: 'core/ingestion/community-processor.ts', asset: 'vendor/leiden/index.cjs' }, +]; + +const escapeRegExp = (s: string): string => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +/** + * Identifiers that behave like `require` in a file: the built-ins plus any + * `const X = createRequire(...)` binding (e.g. `requireCJS`, `_require`). + */ +function requireFamilyIds(content: string): string[] { + const ids = new Set(['require', '_require']); + for (const m of content.matchAll( + /\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*createRequire\s*\(/g, + )) { + ids.add(m[1]); + } + return [...ids]; +} + +/** Net `{` minus `}` before `index`; 0 means the call sits at module top-level. */ +function braceDepthBefore(content: string, index: number): number { + let depth = 0; + for (let i = 0; i < index; i += 1) { + const ch = content[i]; + if (ch === '{') depth += 1; + else if (ch === '}') depth -= 1; + } + return depth; +} + +interface RequireScan { + assets: { asset: string; source: string }[]; + unresolvedComputed: { source: string; arg: string }[]; +} + +/** + * What one source file require()s OUTSIDE `dist/` at module load: + * - `assets`: statically-resolvable relative literals (built-in `require`/ + * `_require`, aliased `createRequire` bindings, and the `createRequire(...) + * ('…')` IIFE) plus the resolved asset of each vetted `KNOWN_COMPUTED_REQUIRES` + * entry. + * - `unresolvedComputed`: MODULE-LOAD (brace-depth 0) requires with a computed / + * non-literal arg that are NOT in the allowlist — the guard fails closed on + * these for manual review. + * The module-load gate applies ONLY to the computed branch: in-function computed + * requires (e.g. `_require(g.pkg)`, `requireCJS(c)`) target `node_modules`/ + * `package.json`, are out of the "module-load" charter, and are ignored. Literal + * requires stay ungated — an in-function `require('../x')` still resolves to the + * same `dist`-relative target, so gating them would only risk dropping real + * coverage. Residual limit: a truly dynamic require whose target is assembled + * across functions / from config is caught only via the fail-closed gate. + */ +function scanContent(relUnderSrc: string, rawContent: string): RequireScan { + const content = stripComments(rawContent); + const distDir = path.posix.join('dist', path.posix.dirname(relUnderSrc)); + const assets: { asset: string; source: string }[] = []; + const unresolvedComputed: { source: string; arg: string }[] = []; + + const addResolved = (spec: string): void => { + const resolved = path.posix.normalize(path.posix.join(distDir, spec)); + if (resolved === 'dist' || resolved.startsWith('dist/')) return; // internal + assets.push({ asset: resolved, source: relUnderSrc }); + }; + + // (a) Literal relative requires: createRequire IIFE … + for (const m of content.matchAll(CREATE_REQUIRE_IIFE_RE)) addResolved(m[1]); + // … plus built-ins and aliased createRequire bindings called with a literal + // (`require('../x')`, `_require('../x')`, `requireCJS('../x')`). + const idAlt = requireFamilyIds(content).map(escapeRegExp).join('|'); + const aliasLiteralRe = new RegExp( + `(?.resolve(...)` is excluded — `\s*\(` must follow the identifier, but + // `.resolve(` sits between, so it never matches (it is a path lookup, not a load). + const computedRe = new RegExp(`(? k.source === relUnderSrc); + if (known) assets.push({ asset: known.asset, source: relUnderSrc }); + else unresolvedComputed.push({ source: relUnderSrc, arg: m[1].trim() }); + } + + return { assets, unresolvedComputed }; +} + +/** + * Aggregate {@link scanContent} over the whole `src/` tree. `src/.ts` + * compiles to `dist/.js`, so a specifier resolved against `dist/` + * reproduces the runtime layout exactly. + */ +function requiredExternalAssets(): RequireScan { + const assets: { asset: string; source: string }[] = []; + const unresolvedComputed: { source: string; arg: string }[] = []; + for (const file of listSourceFiles(SRC_DIR)) { + const relUnderSrc = toPosix(path.relative(SRC_DIR, file)); + const scan = scanContent(relUnderSrc, readFileSync(file, 'utf-8')); + assets.push(...scan.assets); + unresolvedComputed.push(...scan.unresolvedComputed); + } + return { assets, unresolvedComputed }; +} + +/** + * Hand-written shipped assets the runtime image executes directly (hook and + * installer `.cjs`/`.mjs`), derived from the runtime COPY set minus dependency/ + * data roots (`node_modules`, `vendor`, `dist`, `skills`, `package.json`). + * Unlike `src/**` these are not compiled, so they require siblings at their OWN + * package-relative location. + */ +function shippedAssetFiles(copiedSources: string[]): string[] { + const SKIP = new Set(['dist', 'node_modules', 'vendor', 'skills', 'web', 'package.json']); + const out: string[] = []; + const walk = (absDir: string, relDir: string): void => { + for (const e of readdirSync(absDir, { withFileTypes: true })) { + const abs = path.join(absDir, e.name); + const rel = relDir ? `${relDir}/${e.name}` : e.name; + if (e.isDirectory()) walk(abs, rel); + else if (/\.(cjs|mjs|js)$/.test(e.name)) out.push(rel); + } + }; + for (const entry of new Set(copiedSources)) { + if (SKIP.has(entry)) continue; + if (/\.(cjs|mjs|js)$/.test(entry)) { + out.push(entry); // a single shipped script (e.g. scripts/install-duckdb-extension.mjs) + } else { + walk(path.join(GITNEXUS_ROOT, entry), entry); // a directory of shipped assets (e.g. hooks) + } + } + return out; +} + +/** + * Relative `require()`s of shipped `.cjs`/`.mjs` assets, resolved against the + * asset's OWN package-relative dir (not the `dist` mapping). Coverage is checked + * by COPY prefix, never on-disk existence: `hooks/antigravity/*.cjs` does + * `require('./hook-lock.cjs')`, which resolves to `hooks/antigravity/hook-lock.cjs` + * — a path that need not physically exist but IS covered by the whole-`hooks` COPY. + */ +function shippedAssetRequiredAssets(copiedSources: string[]): { asset: string; source: string }[] { + const found: { asset: string; source: string }[] = []; + for (const rel of shippedAssetFiles(copiedSources)) { + const baseDir = path.posix.dirname(rel); + const content = stripComments(readFileSync(path.join(GITNEXUS_ROOT, rel), 'utf-8')); + for (const m of content.matchAll(/\brequire\s*\(\s*['"](\.[^'"]+)['"]\s*\)/g)) { + found.push({ asset: path.posix.normalize(path.posix.join(baseDir, m[1])), source: rel }); + } + } + return found; +} + +describe('Dockerfile.cli runtime-stage asset parity (#2130)', () => { + const dockerfile = readFileSync(DOCKERFILE, 'utf-8'); + const copied = runtimeStageCopiedSources(dockerfile); + + it('parses at least one runtime-stage COPY (guards against a vacuous pass)', () => { + // If the runtime `FROM` or the `/app/gitnexus/` source prefix ever stops + // matching, `copied` goes empty and the parity assertion below would pass + // vacuously (an empty copied set yields zero uncovered assets). Fail loud. + expect(copied.length, 'runtime stage must contain COPY --from=builder lines').toBeGreaterThan( + 0, + ); + }); + + it('copies hooks/ — resolve-invocation.ts require()s it at module load (#2130)', () => { + // The exact regression: without this COPY, `gitnexus analyze` crashes inside + // the image with `Cannot find module '../../hooks/claude/resolve-analyze-cmd.cjs'`. + expect(copied).toContain('hooks'); + }); + + it('copies skills/ — CLI reads the bundled SKILL.md templates at runtime', () => { + // Degradation class (not a crash): `gitnexus analyze --skills` (ai-context.ts) + // and `gitnexus setup`/`uninstall` read `/skills/*.md`. Absent, they + // silently emit placeholder content / install nothing. The image ships it to + // stay fully usable as a CLI. `web/` (also in `files`) is intentionally NOT + // shipped — this image never builds gitnexus-web, so it is API-only. + expect(copied).toContain('skills'); + }); + + it('sanity-checks the scanner sees both literal and vetted-computed module-load deps', () => { + const assets = requiredExternalAssets().assets.map((a) => a.asset); + // literal IIFE require (resolve-invocation.ts → hooks) … + expect(assets).toContain('hooks/claude/resolve-analyze-cmd.cjs'); + // … and the vetted COMPUTED require (community-processor.ts → vendor/leiden). + expect(assets).toContain('vendor/leiden/index.cjs'); + }); + + it('copies every out-of-dist asset reached by a resolvable or vetted module-load require', () => { + // Coverage = statically-resolvable relative literals (built-in / aliased / + // IIFE createRequire) + vetted KNOWN_COMPUTED_REQUIRES; any UNRECOGNIZED + // module-load computed require fails closed for manual review. Truly dynamic + // requires (target assembled across functions / from config) are caught only + // via that fail-closed gate, never statically resolved. + const { assets, unresolvedComputed } = requiredExternalAssets(); + expect( + unresolvedComputed, + `Unverifiable module-load computed require(s) — statically confirm each target is ` + + `COPY'd into the image and add it to KNOWN_COMPUTED_REQUIRES:\n` + + unresolvedComputed.map((u) => ` - src/${u.source}: require(${u.arg})`).join('\n'), + ).toEqual([]); + const uncovered = assets.filter(({ asset }) => !isCovered(asset, copied)); + expect( + uncovered, + `Dockerfile.cli runtime stage is missing COPY lines for module-load require() targets ` + + `outside dist/. Each will crash with MODULE_NOT_FOUND inside the image (cf. #2130). ` + + `Add a \`COPY --from=builder /app/gitnexus/ ./gitnexus/\`:\n` + + uncovered.map((u) => ` - ${u.asset} (required by src/${u.source})`).join('\n'), + ).toEqual([]); + }); + + it('coverage-checks allowlisted computed requires instead of trusting them', () => { + // vendor/leiden is contributed by KNOWN_COMPUTED_REQUIRES. Removing the + // `vendor` COPY must make it surface as uncovered — proving the allowlist + // suppresses only the unresolvable hard-fail, NOT the COPY check (else + // deleting a COPY would silently pass, recreating the #2130 class). + const assets = requiredExternalAssets().assets.map((a) => a.asset); + expect(assets).toContain('vendor/leiden/index.cjs'); + const copiedWithoutVendor = copied.filter((c) => c !== 'vendor'); + expect(isCovered('vendor/leiden/index.cjs', copied)).toBe(true); + expect(isCovered('vendor/leiden/index.cjs', copiedWithoutVendor)).toBe(false); + }); + + it('fails closed on an unrecognized module-load computed require', () => { + const scan = scanContent( + 'fake/widget.ts', + 'const r = createRequire(import.meta.url);\nconst mod = r(somethingComputed);\n', + ); + expect(scan.unresolvedComputed).toHaveLength(1); + expect(scan.unresolvedComputed[0]).toMatchObject({ + source: 'fake/widget.ts', + arg: 'somethingComputed', + }); + }); + + it('resolves aliased createRequire literals and ignores in-function computed requires', () => { + // Aliased binding with a relative literal → resolved like require('../x'). + const aliased = scanContent( + 'cli/widget.ts', + "const requireCJS = createRequire(import.meta.url);\nconst x = requireCJS('../../hooks/z.cjs');\n", + ); + expect(aliased.assets.map((a) => a.asset)).toContain('hooks/z.cjs'); + expect(aliased.unresolvedComputed).toEqual([]); + // A computed require INSIDE a function is out of the module-load charter. + const inFn = scanContent('cli/widget.ts', 'function f(pkg) {\n return require(pkg);\n}\n'); + expect(inFn.unresolvedComputed).toEqual([]); + expect(inFn.assets).toEqual([]); + }); + + it('covers sibling requires of shipped .cjs/.mjs assets (coverage, not existence)', () => { + const shipped = shippedAssetRequiredAssets(copied); + // Sanity: the hand-written hook .cjs sibling requires are actually scanned + // (e.g. gitnexus-hook.cjs → ./hook-lock.cjs); guards against a silent no-op. + expect(shipped.length).toBeGreaterThan(0); + const uncovered = shipped.filter(({ asset }) => !isCovered(asset, copied)); + expect( + uncovered, + `Shipped .cjs/.mjs assets require siblings not COPY'd into the image:\n` + + uncovered.map((u) => ` - ${u.asset} (required by ${u.source})`).join('\n'), + ).toEqual([]); + // The antigravity hook's `require('./hook-lock.cjs')` resolves to a path that + // does NOT physically exist (hook-lock.cjs lives under hooks/claude), yet is + // covered by the whole-`hooks` COPY — coverage-check, not existence-check. + expect(isCovered('hooks/antigravity/hook-lock.cjs', copied)).toBe(true); + }); +}); 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