fix(hooks): silence MCP-owned-DB augment skip for strict hook runners (#1913)

[magyargergo]

Summary

Fixes #1913. The GitNexus PreToolUse hook's MCP-owned-DB skip path wrote [GitNexus] augment skipped: MCP server owns DB to stderr unconditionally on a normal (non-error) skip. Strict hook runners that validate hook output — e.g. Codex PreToolUse — treat this as noisy / hook returned invalid pre-tool-use JSON output.

This makes the diagnostic debug-only: normal skips are now silent by default (empty stdout and stderr, exit 0), and the reason stays recoverable with GITNEXUS_DEBUG=1.

Root cause

handlePreToolUse → the hasGitNexusServerOwner(...) branch was the only ungated process.stderr.write on a normal Pre/PostToolUse path. (The DB-lock probe fails closed — an lsof ETIMEDOUT also routes through this same line — so the fail-closed skip is now silent by default too, which is the correct strict-runner trade-off.)

The fix

• Added a shared isDebugEnabled() helper (GITNEXUS_DEBUG === '1' || === 'true') and gated the skip diagnostic behind it. Refactored each extractAugmentContext to reuse the helper (DRY).
• Applied consistently to all three hand-maintained hook copies (found via repo-wide grep, not just the one named in the issue):
  • gitnexus/hooks/claude/gitnexus-hook.cjs (canonical)
  • gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs
  • gitnexus-claude-plugin/hooks/gitnexus-hook.js
• The Cursor hook has no MCP-owned-DB skip path and is correctly out of scope. No generator/template emits these files, so hand-edits are the source of truth (setup.ts only rewrites a cliPath literal at install time).

Testing

• Unit (test/unit/hooks.test.ts, claude CJS + plugin): the canonical owner test now asserts default-silent (empty stdout + stderr, exit 0, GITNEXUS_DEBUG forced off for determinism); added a paired debug-on test asserting the message reappears under GITNEXUS_DEBUG=1; the 4 owner-detection tests run with GITNEXUS_DEBUG=1 so the skip discriminator stays observable.
• e2e (test/integration/antigravity-hook-e2e.test.ts): the antigravity adapter shares the identical gated skip but only runs from its install dir (its lock/probe helpers are install-time-copied), so it's covered through the install pipeline with a faked DB-owner probe — default-silent + debug-on. The fake-probe helpers (createHookToolDir / hookEnv / writeExecutable) were promoted into shared hook-test-helpers so unit + e2e reuse them.

All 155 unit + 22 antigravity e2e tests pass; tsc clean on the edited files.

Review

Self-reviewed via a multi-agent adversarial pass (correctness / completeness / tests / project-standards), each finding independently verified. The only confirmed actionable finding was the missing antigravity behavioral coverage, now added here.

🤖 Generated with Claude Code

[vercel]

@magyargergo is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

CI Report

✅ All checks passed

Pipeline Status

Stage	Status	Details
✅ Typecheck	success	tsc --noEmit
✅ Tests	success	unit tests, 3 platforms
✅ E2E	success	gitnexus-web changes only

Test Results

Tests	Passed	Failed	Skipped	Duration
10890	10874	0	16	561s

✅ All 10874 tests passed

16 test(s) skipped — expand for details

• COBOL pipeline benchmark > scales with file count
• C++ ADL emit benchmark > emit phase scales sub-quadratically with co-scaled files and sites
• C++ pipeline benchmark > scales with file count
• C# pipeline benchmark > scales with file count — namespaces spread across the solution
• C# pipeline benchmark > scales with file count — all types in one (global) namespace bucket
• C# pipeline benchmark > scales with file count — all types in one (named) namespace bucket
• Go pipeline benchmark > scales with file count (workers enabled)
• Go pipeline benchmark — worker pool (issue Worker idle timeout kills long Go scope extraction and surfaces as Napi::Error during analyze #1848) > does not quarantine the large generated Go file on sub-batch idle timeout
• Go structural interface detection benchmark > scales linearly with interface × struct count
• Go structural interface detection split-phase benchmark > separates index-build and detection time
• PHP pipeline benchmark > scales with file count (workers enabled)
• Ruby pipeline benchmark > scales with file count (workers enabled)
• Rust pipeline benchmark > scales with file count (workers enabled)
• Vue pipeline benchmark > scales with component count
• run.cjs direct-exec entrypoint (fix(cli): steer docs, skills, and hooks through a CLI-neutral project-local runner (#1939) #1945) > resolves a .cmd shim via the Windows shell branch, passing args and exit code
• buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric	Coverage	Covered	Base	Delta	Status
Statements	75.08%	35498/47276	N/A%	—	🟢 ███████████████░░░░░
Branches	62.84%	21938/34909	N/A%	—	🟢 ████████████░░░░░░░░
Functions	80.83%	3834/4743	N/A%	—	🟢 ████████████████░░░░
Lines	78.88%	32098/40691	N/A%	—	🟢 ███████████████░░░░░

---

_{📋 View full run · Generated by CI}

[magyargergo]

Tri-review: PR #2134 — fix(hooks): silence MCP-owned-DB augment skip (#1913)

Methods (engine breakdown). 6 Claude reviewer lanes — risk-architect, test/CI (returned partial), correctness, adversarial, maintainability, testing — plus Codex (codex-rescue), the one genuinely independent engine, which ran live and returned structured findings. Two of the three method-families are Claude reading the same diff under different personas, so Claude-only agreement is "consistent across personas," not independent corroboration; only Codex + a Claude lane agreeing is treated as strong. This digest was gated by an independent synthesis-critic before posting.

Verdict: no P0/P1. The change gates a single non-error stderr write behind a shared isDebugEnabled() helper across all three hand-maintained hook copies; the runtime contract (stdout hookSpecificOutput JSON) is untouched. Correctness confirmed the new gate is semantically identical to the old inline check and that no normal hook path still emits ungated output; adversarial could not construct a break (the new tests are non-vacuous — deleting the gate turns stderr.trim()==='' red); risk rated blast radius LOW.

Findings (all P2/P3) — actionable ones already addressed in 456a0e98:

• [P3 · Codex + testing] Antigravity e2e default-silent test used a weaker assertion (not.toContain / parseHookOutput===null) than the unit suite's strict stderr.trim()==='' / stdout.trim()===''. → Fixed: tightened to the strict empty-output checks (re-ran: the install-pipeline owner-skip path is genuinely clean, so the strict assertion holds).
• [P2 · risk + testing] The fail-closed (lsof ETIMEDOUT) skip routes through the same gated line but had only a debug-on test, asymmetric with the MCP-owner path. → Fixed: added a paired GITNEXUS_DEBUG='' ETIMEDOUT-silent test (CJS + Plugin).
• [P2 · maintainability] writeExecutable was exported from the shared test helper with no external consumer. → Fixed: made module-private.
• [P3 · maintainability] lbugPath wasn't cleaned in the two new unit tests' finally (the new e2e tests do). → Fixed: added cleanup for consistency.

Considered and deliberately not changed (with rationale):

• [P3 · Codex + risk + correctness] The main() catch-handler still uses truthy if (process.env.GITNEXUS_DEBUG) while the new helper is strict (=== '1' || === 'true'). All three lanes agree this is pre-existing and not a regression. Left as-is: the catch handler logs genuine exceptions, where a more permissive gate is defensible (you want crash diagnostics even under non-canonical debug values); unifying it to strict would narrow error logging — the wrong direction — and is outside #1913's scope.
• GITNEXUS_DEBUG undocumented for the claude/antigravity hooks (P3), and the antigravity AfterTool stale-index hint writing ungated to stderr (P3, pre-existing #1730, an intentional terminal mirror). Both pre-existing and broader than this fix — noted as follow-ups, not addressed here.

CI. Core code checks green at review time (lint, typecheck, typecheck-web, gitleaks, review, tree-sitter ABI); the full test matrix + image builds were still in flight. Vercel = deploy-auth, not a code failure. 179 unit + e2e tests pass locally; prettier + tsc clean.

---

Automated multi-tool digest (GitNexus swarm + Compound-Engineering personas + Codex), vetted by a synthesis-critic. Verify before acting — only Codex is an independent engine; the rest share Claude's priors.