diff --git a/.factory-plugin/marketplace.json b/.factory-plugin/marketplace.json new file mode 100644 index 000000000..3fb30e1e4 --- /dev/null +++ b/.factory-plugin/marketplace.json @@ -0,0 +1,19 @@ +{ + "name": "gitnexus-marketplace", + "owner": { + "name": "GitNexus", + "email": "nico@gitnexus.dev" + }, + "metadata": { + "description": "Code intelligence powered by a knowledge graph — execution flows, blast radius, and semantic search", + "homepage": "https://github.com/nicosxt/gitnexus" + }, + "plugins": [ + { + "name": "gitnexus", + "version": "1.6.12", + "source": "./gitnexus-factory-plugin", + "description": "Code intelligence powered by a knowledge graph. Provides execution flow tracing, blast radius analysis, and augmented search across your codebase." + } + ] +} diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index ca81460f1..57f50c4fd 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -692,7 +692,20 @@ jobs: git add ../gitnexus-claude-plugin/.claude-plugin/plugin.json \ ../.claude-plugin/marketplace.json \ ../gitnexus-claude-plugin/.codex-plugin/plugin.json \ - ../.agents/plugins/marketplace.json + ../.agents/plugins/marketplace.json \ + ../gitnexus-factory-plugin/.factory-plugin/plugin.json \ + ../gitnexus-factory-plugin/mcp.json \ + ../.factory-plugin/marketplace.json \ + ../gitnexus-claude-plugin/skills/gitnexus-plan/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-work/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-review/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-lfg/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-guide/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-cli/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-debugging/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-exploring/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-impact-analysis/mcp.json \ + ../gitnexus-claude-plugin/skills/gitnexus-refactoring/mcp.json git commit -m "release: ${VTAG}" --allow-empty RELEASE_SHA="$(git rev-parse HEAD)" echo "Detached release commit: $RELEASE_SHA" diff --git a/README.md b/README.md index e890aa599..9d919aa1c 100644 --- a/README.md +++ b/README.md @@ -233,12 +233,13 @@ When a repo contains an `.agents/` directory, the standard and generated skills | **Cursor** | Yes | Yes | Yes (postToolUse, [manual install](gitnexus-cursor-integration/README.md#hook-install)) | **Full** | | **Antigravity** (Google) | Yes | Yes | Yes (AfterTool, [Gemini CLI hooks schema](https://geminicli.com/docs/hooks/reference/))[¹](#fn-antigravity-hooks) | **Full** | | **Codex** | Yes | Yes | Yes (PreToolUse + PostToolUse, [Codex hooks](https://developers.openai.com/codex/hooks)) | **Full** | +| **Factory** (Droid) | Yes | Yes | Yes (PostToolUse, [plugin](gitnexus-factory-plugin/)) | **Full** | | **OpenCode** | Yes | Yes | — | MCP + Skills | | **CodeBuddy** (Tencent) | Yes | Yes | — | MCP + Skills | | **Qoder** (Alibaba) | Yes | Yes | — | MCP + Skills | | **Windsurf** | Yes | — | — | MCP | -> **Claude Code** and **Codex** get the deepest integration: MCP tools + agent skills + PreToolUse hooks that enrich searches with graph context + PostToolUse hooks that detect a stale index after commits and prompt the agent to reindex. +> **Full** means MCP tools + agent skills + hooks that enrich searches with graph context. **Claude Code** and **Codex** go deepest: their PreToolUse hooks enrich the search before it runs, and their PostToolUse hooks also detect a stale index after commits and prompt the agent to reindex. **Cursor**, **Antigravity**, and **Factory** augment from a post-tool hook only, so they enrich the result rather than the query and do not carry the stale-index hint. @@ -282,6 +283,21 @@ codex plugin marketplace add abhigyanpatwari/GitNexus > **Codex notes:** SessionStart is intentionally not registered — Codex reads [AGENTS.md natively](https://developers.openai.com/codex/guides/agents-md), which already carries the GitNexus context block. Newly installed hooks need a one-time approval in Codex via `/hooks` before they run. Pick **one** install route (`gitnexus setup -c codex` **or** the plugin): plugin hooks load alongside `~/.codex/hooks.json`, so installing both can fire duplicate hooks per tool call. +**Factory** (Droid) — MCP + skills via `gitnexus setup -c droid`, or add the server manually to `~/.factory/mcp.json` ([user scope](https://docs.factory.ai/cli/configuration/mcp), applies to all projects): + +```json +{ + "mcpServers": { + "gitnexus": { + "command": "npx", + "args": ["-y", "gitnexus@latest", "mcp"] + } + } +} +``` + +`gitnexus setup -c droid` also installs skills to `~/.factory/skills/`. For the PostToolUse search-augment hook, install the bundled [`gitnexus-factory-plugin/`](gitnexus-factory-plugin/) — from a marketplace that includes this repo, run `droid plugin install gitnexus@`, or point Droid at it via `extraKnownMarketplaces` in `.factory/settings.json`. Factory reads [`AGENTS.md` natively](https://docs.factory.ai/), which already carries the GitNexus context block. + **Cursor** (`~/.cursor/mcp.json` — global, works for all projects): ```json diff --git a/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs b/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs index 4376795e2..3e98f5901 100644 --- a/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs +++ b/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs @@ -80,9 +80,11 @@ function debugLog(msg) { function resolveHookBinary(tool) { const envKey = tool === 'lsof' ? 'GITNEXUS_HOOK_LSOF_PATH' : 'GITNEXUS_HOOK_PS_PATH'; - const fromEnv = process.env[envKey]; - if (fromEnv && String(fromEnv).trim() && fs.existsSync(String(fromEnv))) { - return String(fromEnv); + // Trim once, exactly as hasMissingHookBinaryOverride does, so a padded but + // valid override (" /tmp/lsof ") is both accepted there and used here. + const fromEnv = process.env[envKey] ? String(process.env[envKey]).trim() : ''; + if (fromEnv && fs.existsSync(fromEnv)) { + return fromEnv; } const candidates = tool === 'lsof' @@ -177,7 +179,13 @@ function resolveUnixGuardTimeout() { const trimmed = fromEnv ? String(fromEnv).trim() : ''; if (trimmed === 'disabled') return unixGuardTimeoutCache; const candidates = []; - if (trimmed && fs.existsSync(trimmed)) candidates.push(trimmed); + // Resolve the override against THIS process's cwd — the directory the + // existsSync check and the self-test run in — so the cached/returned path is + // always absolute. The adapters spawn the wrapper with a different `cwd` + // (the tool request's), where a relative value would resolve elsewhere + // (ENOENT), and a slashless name would switch to a PATH lookup. + const override = trimmed ? path.resolve(trimmed) : ''; + if (override && fs.existsSync(override)) candidates.push(override); for (const builtin of [ '/usr/bin/timeout', '/bin/timeout', @@ -317,15 +325,30 @@ function getProcRoot() { // `node mcp` line // (the `mcp`/`serve` mode token lives at the very tail, so the cap must be large // enough to reach it — see PROC_CMDLINE_FLOOR escalation below). Overridable for -// tests; never goes below PROC_CMDLINE_FLOOR. +// tests via GITNEXUS_HOOK_PROC_CMDLINE_MAX: an integer in +// [PROC_CMDLINE_FLOOR, PROC_CMDLINE_CEIL] is used as-is; a larger integer is +// CLAMPED to PROC_CMDLINE_CEIL; anything else (below the floor, fractional, +// non-numeric, Infinity) falls back to the 16 KiB default. const PROC_CMDLINE_FLOOR = 4096; +// Upper bound for a single cmdline read chunk, and the absolute ceiling of the +// escalation path in readLinuxCmdline (same 256 KiB — no single read may exceed +// what the whole escalation is allowed to collect). Without it, an oversized +// override (e.g. 2**40 — past buffer.constants.MAX_LENGTH on older Node lines +// and unallocatable in practice on any) made Buffer.allocUnsafe throw; readLinuxCmdline's catch turned that into '' (a +// NON-candidate), so a real server owner was silently missed (fail-OPEN, the +// #1492 race). Oversized values are clamped rather than defaulted: the operator +// asked for MORE bytes, and the ceiling is the most the read will ever collect +// anyway, so clamping honours the intent while keeping allocation bounded. +const PROC_CMDLINE_CEIL = 262144; function getCmdlineMaxBytes() { const raw = process.env.GITNEXUS_HOOK_PROC_CMDLINE_MAX; // Number() (not parseInt) so "8e3" reads as 8000, not 8 (parseInt stops at // 'e'). The `raw && String(raw).trim()` guard keeps empty/whitespace on the // default; trailing garbage ("8abc") now -> NaN -> default (stricter). const n = raw && String(raw).trim() ? Number(String(raw).trim()) : NaN; - if (Number.isFinite(n) && n >= PROC_CMDLINE_FLOOR) return n; + // Number.isInteger rejects NaN, +/-Infinity and fractions (a fractional + // Buffer/readSync length is not a byte count). + if (Number.isInteger(n) && n >= PROC_CMDLINE_FLOOR) return Math.min(n, PROC_CMDLINE_CEIL); return 16384; } @@ -381,19 +404,24 @@ const CMDLINE_TIMEOUT = Symbol('gitnexus.cmdline.timeout'); // Bounded /proc//cmdline read for Phase 1. openSync+readSync (not // readFileSync) so a D-state holder cannot stall the hook on a huge or -// never-EOF argv: we read at most `cap` bytes and stop. cmdline separates argv -// with NULs; convert to spaces for isGitNexusServerCommand. +// never-EOF argv: we read in `cap`-sized chunks and stop as soon as the text +// holds both server tokens, at EOF, at PROC_CMDLINE_CEIL, or when the scan +// budget runs out (see below). cmdline separates argv with NULs; convert to +// spaces for isGitNexusServerCommand. // // Owner-miss guard for the 4 KB cap: the `gitnexus` token usually sits in the // first path component while the `mcp`/`serve` mode token is the LAST argv, so // a naive 4 KB read could clip the mode token off a server launched with a very // long interpreter path and silently miss a real owner. We mitigate two ways: -// (a) the default cap (16 KiB) already clears realistic lines; (b) if the first -// read fills the cap AND already contains the `gitnexus` token but no mode -// token yet, we keep reading in bounded chunks (up to a hard ceiling) until the -// mode token appears or the file ends — so a genuine server is never missed for -// want of a few more bytes, while non-candidates still pay only the initial -// bounded read. +// (a) the default cap (16 KiB) already clears realistic lines, so almost every +// process is decided by the first read hitting EOF; (b) a read that fills the +// cap stops early ONLY once it holds BOTH tokens (decided owner). Holding one +// token, or neither, decides nothing: interpreter flags can put a mode-looking +// word first (`node --require mcp .../gitnexus/... serve`) or push the gitnexus +// path past the first chunk. So we keep reading in cap-sized chunks until both +// tokens appear, the file ends, or the hard ceiling is reached. Only processes +// that passed the Phase 0 comm prefilter AND have a cmdline longer than the cap +// ever escalate, and each escalation step is budget-gated (below). // // Budget (F3): the escalation loop above is the one place a SINGLE pathological // candidate could read up to HARD_CEIL (256 KiB) before the next scan-level @@ -411,7 +439,7 @@ function readLinuxCmdline(procRoot, pidStr, cap, outOfBudget) { return ''; } try { - const HARD_CEIL = 262144; // 256 KiB absolute ceiling for the escalation path + const HARD_CEIL = PROC_CMDLINE_CEIL; // 256 KiB absolute ceiling for the escalation path let collected = Buffer.alloc(0); let offset = 0; let chunkCap = cap; @@ -425,16 +453,12 @@ function readLinuxCmdline(procRoot, pidStr, cap, outOfBudget) { collected = Buffer.concat([collected, buf.subarray(0, bytes)]); offset += bytes; const text = collected.toString('utf8').replace(/\0+/g, ' '); - // Stop early when we can already decide "owner": has both the gitnexus - // token and a mode token. Keep going only when gitnexus is present but - // the mode token might be just past the boundary. - const hasGitNexus = - /(?:^|[/\\\s])gitnexus(?:\.cmd)?(?:\s|$)/.test(text) || - /node_modules[/\\]gitnexus[/\\]/.test(text); - const hasMode = /(?:^|\s)(mcp|serve)(?:\s|$)/.test(text); - if (hasMode) break; // decided (positive); isGitNexusServerCommand re-checks below + // Stop early only when the partial read is DECIDED: both the gitnexus + // token and a mode token are present (isGitNexusServerCommand is exactly + // that conjunction). A partial read missing either token is undecided — + // the missing one may lie past the chunk boundary — so it keeps reading. + if (isGitNexusServerCommand(text)) break; // decided (positive) if (bytes < chunkCap) break; // EOF: full cmdline read, definitive - if (!hasGitNexus) break; // not a candidate; do not escalate the read if (offset >= HARD_CEIL) break; // bounded escalation only // Budget gate the escalation: a single huge-argv candidate must not burn // the whole scan deadline before we re-check. Return the timeout sentinel @@ -578,17 +602,24 @@ function linuxProcScanFindGitNexusServer(dbPathAbs, myPid) { ); return 'timeout'; } - // Any other shape (ENOTDIR — fd path is not a directory at all, so this - // is not a plausible live-procfs owner — and the long tail) is treated as - // "this candidate is not an owner": move to the next candidate instead of - // the old blanket 'owned'. If no other candidate owns the lbug the scan - // ends not-owned (dispatcher fail-open) — acceptable because ENOTDIR means - // the fd entry is structurally not a real /proc//fd. + if (code === 'ENOTDIR') { + // The fd path is not a directory at all, so this is structurally not a + // real /proc//fd — not a plausible live owner. Move on. + debugLog( + `fd dir not a directory for candidate pid ${pidStr} (ENOTDIR); ` + + `treating candidate as non-owner -> continue`, + ); + continue; + } + // Any other error (EMFILE, ENFILE, ENOMEM, EINTR, no code, …) says nothing + // about whether this already-identified server candidate holds the lbug. + // Only ENOENT/ENOTDIR above establish non-ownership; everything else is + // inconclusive and fails closed via 'timeout', same as EACCES/EIO. debugLog( - `fd dir not a readable directory for candidate pid ${pidStr} ` + - `(${code || 'unknown'}); treating candidate as non-owner -> continue`, + `fd dir read failed for candidate pid ${pidStr} ` + + `(${code || 'unknown'}); probe inconclusive -> fail-closed (timeout)`, ); - continue; + return 'timeout'; } for (const fd of fds) { if (outOfBudget()) return 'timeout'; @@ -707,16 +738,12 @@ module.exports = { // name is pinned by a source-contract test. linuxProcScanFindGitNexusServer, // #2163 follow-up: the hook adapters wrap the augment CLI in the same - // guard. Returns a self-tested wrapper path — the built-in candidates are - // always absolute; a GITNEXUS_HOOK_TIMEOUT_PATH override is adopted as the - // exact string that passed the self-test. Same string is also the same - // RESOLUTION for absolute paths and for slashless names (PATH lookup is - // cwd-independent); a slash-containing RELATIVE override, however, is - // existsSync-checked and self-tested against this process's cwd while the - // adapters spawn the CLI with a `cwd` option (chdir-before-exec), so such - // a value can pass here yet ENOENT at the augment call site — set the - // override to an absolute path. Returns null when the wrapper is - // disabled/unavailable. Never call on win32 (see its JSDoc). + // guard. Returns a self-tested, always-ABSOLUTE wrapper path: the built-in + // candidates are absolute, and a GITNEXUS_HOOK_TIMEOUT_PATH override is + // path.resolve()d against this process's cwd before its existsSync check + // and self-test, so the adapters can spawn it under any `cwd` option. + // Returns null when the wrapper is disabled/unavailable. Never call on + // win32 (see its JSDoc). resolveUnixGuardTimeout, // Exported for white-box unit tests of the numeric-env parsing (#2183 review): // Number()-not-parseInt so "16e3" reads as 16000, plus the empty/whitespace @@ -725,4 +752,8 @@ module.exports = { // otherwise only observable indirectly through scan timing/escalation. getCmdlineMaxBytes, resolveLinuxProcBudgetMs, + // Exported for white-box tests pinning that the override check and the + // override lookup agree on whitespace-padded GITNEXUS_HOOK_{LSOF,PS}_PATH. + resolveHookBinary, + hasMissingHookBinaryOverride, }; diff --git a/gitnexus-claude-plugin/hooks/hook-lock.js b/gitnexus-claude-plugin/hooks/hook-lock.js index 759856384..05ee20ace 100644 --- a/gitnexus-claude-plugin/hooks/hook-lock.js +++ b/gitnexus-claude-plugin/hooks/hook-lock.js @@ -1,3 +1,4 @@ +const crypto = require('crypto'); const fs = require('fs'); const path = require('path'); @@ -5,6 +6,128 @@ const HOOK_LOCK_SUBDIR = '.hook-locks'; const HOOK_LOCK_MAX_INFLIGHT = 3; const HOOK_LOCK_STALE_MS = 30000; +// An evictor's claim marker older than this belongs to a crashed evictor. +// The critical section it guards is a few syscalls (token read, lstat, +// unlink), so any live evictor finishes orders of magnitude sooner; kept well +// under HOOK_LOCK_STALE_MS so an orphan never blocks a slot for long. +const HOOK_LOCK_EVICT_MARKER_STALE_MS = 5000; + +// Same file iff inode identity AND content metadata match. dev+ino alone is +// not enough: filesystems reuse a freed inode number immediately (ext4), so a +// file recreated after an unlink can carry the old file's ino. bigint stats +// keep Windows' 64-bit file ids exact. +function sameSlotFile(a, b) { + return a.dev === b.dev && a.ino === b.ino && a.size === b.size && a.mtimeNs === b.mtimeNs; +} + +function readMarkerToken(marker) { + try { + return fs.readFileSync(marker, 'utf-8'); + } catch { + return null; + } +} + +// Stat and token of a marker, both taken from one open descriptor so they +// describe the same file (a path stat followed by a path read could straddle +// a replacement). O_NOFOLLOW where the platform has it: a marker is always a +// regular file this module created. Returns null when there is no marker. +function readMarkerSnapshot(marker) { + let fd; + try { + fd = fs.openSync(marker, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); + return { stat: fs.fstatSync(fd, { bigint: true }), token: fs.readFileSync(fd, 'utf-8') }; + } catch { + return null; + } finally { + if (fd !== undefined) { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + } + } +} + +// Break an evictor's claim marker only if it is an orphan: older than +// HOOK_LOCK_EVICT_MARKER_STALE_MS, and still the exact file (identity and +// owner token) judged old when it is re-checked just before the unlink. A +// marker released and re-created by a new claimant in between is fresh, so +// it fails the check and stays. +function breakOrphanedMarker(marker) { + const seen = readMarkerSnapshot(marker); + if (!seen || Date.now() - Number(seen.stat.mtimeMs) <= HOOK_LOCK_EVICT_MARKER_STALE_MS) return; + const now = readMarkerSnapshot(marker); + if (!now || !sameSlotFile(now.stat, seen.stat) || now.token !== seen.token) return; + try { + fs.unlinkSync(marker); + } catch { + /* another contender already cleared it */ + } +} + +// Evict a slot judged stale from the `inspected` stat. A slot file is only +// ever deleted, never moved, and only by the evictor holding the per-slot +// `.evicting` marker, created O_EXCL with a token unique to this call. +// Every destructive step verifies first: +// - the slot is unlinked only if the marker still carries our token (an +// evictor stalled long enough for its marker to be broken as an orphan has +// lost its claim and backs off) and the slot is still the exact file +// inspected — identical dev/ino/size/mtimeNs means its content and age are +// unchanged, so the stale verdict still holds, while a slot recreated since +// inspection fails the check and its lock stands; +// - our marker is removed only if it still carries our token, so a marker +// that has passed to another claimant is left alone; +// - an orphaned marker is broken only if it is still the old file it was +// judged to be (see breakOrphanedMarker). +// +// Residual windows. POSIX has no conditional unlink, so each check-then- +// unlink pair keeps a gap of two adjacent syscalls: +// (a) Slot: between the lstat identity check and unlinkSync(slot), a live +// owner past HOOK_LOCK_STALE_MS could release and a new hook recreate the +// slot, whose fresh lock would then be deleted. The consequence is at +// most one extra concurrent augment beyond HOOK_LOCK_MAX_INFLIGHT for +// that run — the cap is a load guard, and no data or index state +// depends on it. The victim's release() sees a foreign or missing file +// and leaves it alone. +// (b) Marker: between the token re-read and unlinkSync(marker) (ours or an +// orphan's), the marker could pass to another claimant, whose claim would +// then be removed. That only re-opens the slot to one more evictor, which +// still has to pass the slot identity check before deleting anything. +// Both need a stall of seconds landing on that exact syscall pair, and the +// only thing lost is one run's cap accounting, so they are accepted rather +// than traded for heavier machinery. A crash at any point orphans at most the +// marker, which the next contender breaks after it expires. +function evictStaleSlot(slotPath, inspected) { + const marker = `${slotPath}.evicting`; + breakOrphanedMarker(marker); + const token = `${process.pid}:${crypto.randomBytes(8).toString('hex')}`; + try { + fs.writeFileSync(marker, token, { flag: 'wx' }); + } catch { + return; // Another evictor holds this slot — leave it to that evictor. + } + try { + if ( + readMarkerToken(marker) === token && + sameSlotFile(fs.lstatSync(slotPath, { bigint: true }), inspected) + ) { + fs.unlinkSync(slotPath); + } + } catch { + /* slot already gone — the retry claims it */ + } finally { + if (readMarkerToken(marker) === token) { + try { + fs.unlinkSync(marker); + } catch { + /* already gone */ + } + } + } +} + function acquireHookSlot(gitNexusDir) { const lockDir = path.join(gitNexusDir, HOOK_LOCK_SUBDIR); try { @@ -28,6 +151,7 @@ function acquireHookSlot(gitNexusDir) { const release = () => { if (released) return; released = true; + process.removeListener('exit', release); try { // Only unlink if we still own the slot. If we appeared stale and // another hook took over, the file now belongs to it — leave alone. @@ -52,8 +176,10 @@ function acquireHookSlot(gitNexusDir) { } let isLive = false; let mtimeMs = Date.now(); + let inspected = null; try { - mtimeMs = fs.fstatSync(fd).mtimeMs; + inspected = fs.fstatSync(fd, { bigint: true }); + mtimeMs = Number(inspected.mtimeMs); const buf = Buffer.alloc(32); const n = fs.readSync(fd, buf, 0, 32, 0); const ownerStr = buf.slice(0, n).toString('utf-8').trim(); @@ -98,11 +224,9 @@ function acquireHookSlot(gitNexusDir) { isLive = false; } if (isLive) break; // Try the next slot. - try { - fs.unlinkSync(slotPath); - } catch { - /* another hook beat us to it — retry will hit EEXIST */ - } + // No stat means we cannot prove which file we judged stale; leave it + // (the retry re-inspects it) rather than risk deleting a fresh lock. + if (inspected) evictStaleSlot(slotPath, inspected); // Loop and retry this slot. } } diff --git a/gitnexus-claude-plugin/hooks/registry-query.cjs b/gitnexus-claude-plugin/hooks/registry-query.cjs index f5b2a0576..0607602db 100644 --- a/gitnexus-claude-plugin/hooks/registry-query.cjs +++ b/gitnexus-claude-plugin/hooks/registry-query.cjs @@ -248,40 +248,35 @@ function storageSlotName(repoPath) { return `${basename}-${digest}`; } +// Definedness matches the CLI's storage-resolver.ts: any value other than +// undefined (including '') counts as configured. function envOverridesStorage() { - const envPath = process.env[STORAGE_PATH_ENV]; - const envRoot = process.env[STORAGE_ROOT_ENV]; - return ( - (typeof envPath === 'string' && envPath.length > 0) || - (typeof envRoot === 'string' && envRoot.length > 0) - ); + return process.env[STORAGE_PATH_ENV] !== undefined || process.env[STORAGE_ROOT_ENV] !== undefined; } +// A set-but-invalid override (empty, relative, or containing NUL) makes +// storage unresolvable (the CLI's storage-resolver.ts throws); never fall +// back to the registry row. A filesystem root is invalid only for +// GITNEXUS_STORAGE_PATH (validateConfiguredStoragePath rejects it); +// GITNEXUS_STORAGE_ROOT accepts a filesystem root — storagePathFromRoot +// resolves the slot directly under it. function resolveEntryStoragePath(entry) { const envPath = process.env[STORAGE_PATH_ENV]; - if ( - typeof envPath === 'string' && - envPath.length > 0 && - !envPath.includes('\0') && - path.isAbsolute(envPath) - ) { + if (envPath !== undefined) { + if (!envPath || envPath.includes('\0') || !path.isAbsolute(envPath)) return null; const resolved = path.resolve(envPath); - if (path.isAbsolute(resolved)) return resolved; + // validateConfiguredStoragePath rejects a filesystem root. + return path.basename(resolved) ? resolved : null; } const envRoot = process.env[STORAGE_ROOT_ENV]; - if ( - typeof envRoot === 'string' && - envRoot.length > 0 && - !envRoot.includes('\0') && - path.isAbsolute(envRoot) - ) { + if (envRoot !== undefined) { + if (!envRoot || envRoot.includes('\0') || !path.isAbsolute(envRoot)) return null; const root = path.resolve(envRoot); const slot = storageSlotName(entry.path); - if (slot) { - const storagePath = path.join(root, slot); - if (samePath(path.dirname(storagePath), root)) return storagePath; - } + if (!slot) return null; + const storagePath = path.join(root, slot); + return samePath(path.dirname(storagePath), root) ? storagePath : null; } if (entry.storagePath !== undefined) { diff --git a/gitnexus-cursor-integration/hooks/hook-lock.cjs b/gitnexus-cursor-integration/hooks/hook-lock.cjs index 759856384..05ee20ace 100644 --- a/gitnexus-cursor-integration/hooks/hook-lock.cjs +++ b/gitnexus-cursor-integration/hooks/hook-lock.cjs @@ -1,3 +1,4 @@ +const crypto = require('crypto'); const fs = require('fs'); const path = require('path'); @@ -5,6 +6,128 @@ const HOOK_LOCK_SUBDIR = '.hook-locks'; const HOOK_LOCK_MAX_INFLIGHT = 3; const HOOK_LOCK_STALE_MS = 30000; +// An evictor's claim marker older than this belongs to a crashed evictor. +// The critical section it guards is a few syscalls (token read, lstat, +// unlink), so any live evictor finishes orders of magnitude sooner; kept well +// under HOOK_LOCK_STALE_MS so an orphan never blocks a slot for long. +const HOOK_LOCK_EVICT_MARKER_STALE_MS = 5000; + +// Same file iff inode identity AND content metadata match. dev+ino alone is +// not enough: filesystems reuse a freed inode number immediately (ext4), so a +// file recreated after an unlink can carry the old file's ino. bigint stats +// keep Windows' 64-bit file ids exact. +function sameSlotFile(a, b) { + return a.dev === b.dev && a.ino === b.ino && a.size === b.size && a.mtimeNs === b.mtimeNs; +} + +function readMarkerToken(marker) { + try { + return fs.readFileSync(marker, 'utf-8'); + } catch { + return null; + } +} + +// Stat and token of a marker, both taken from one open descriptor so they +// describe the same file (a path stat followed by a path read could straddle +// a replacement). O_NOFOLLOW where the platform has it: a marker is always a +// regular file this module created. Returns null when there is no marker. +function readMarkerSnapshot(marker) { + let fd; + try { + fd = fs.openSync(marker, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); + return { stat: fs.fstatSync(fd, { bigint: true }), token: fs.readFileSync(fd, 'utf-8') }; + } catch { + return null; + } finally { + if (fd !== undefined) { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + } + } +} + +// Break an evictor's claim marker only if it is an orphan: older than +// HOOK_LOCK_EVICT_MARKER_STALE_MS, and still the exact file (identity and +// owner token) judged old when it is re-checked just before the unlink. A +// marker released and re-created by a new claimant in between is fresh, so +// it fails the check and stays. +function breakOrphanedMarker(marker) { + const seen = readMarkerSnapshot(marker); + if (!seen || Date.now() - Number(seen.stat.mtimeMs) <= HOOK_LOCK_EVICT_MARKER_STALE_MS) return; + const now = readMarkerSnapshot(marker); + if (!now || !sameSlotFile(now.stat, seen.stat) || now.token !== seen.token) return; + try { + fs.unlinkSync(marker); + } catch { + /* another contender already cleared it */ + } +} + +// Evict a slot judged stale from the `inspected` stat. A slot file is only +// ever deleted, never moved, and only by the evictor holding the per-slot +// `.evicting` marker, created O_EXCL with a token unique to this call. +// Every destructive step verifies first: +// - the slot is unlinked only if the marker still carries our token (an +// evictor stalled long enough for its marker to be broken as an orphan has +// lost its claim and backs off) and the slot is still the exact file +// inspected — identical dev/ino/size/mtimeNs means its content and age are +// unchanged, so the stale verdict still holds, while a slot recreated since +// inspection fails the check and its lock stands; +// - our marker is removed only if it still carries our token, so a marker +// that has passed to another claimant is left alone; +// - an orphaned marker is broken only if it is still the old file it was +// judged to be (see breakOrphanedMarker). +// +// Residual windows. POSIX has no conditional unlink, so each check-then- +// unlink pair keeps a gap of two adjacent syscalls: +// (a) Slot: between the lstat identity check and unlinkSync(slot), a live +// owner past HOOK_LOCK_STALE_MS could release and a new hook recreate the +// slot, whose fresh lock would then be deleted. The consequence is at +// most one extra concurrent augment beyond HOOK_LOCK_MAX_INFLIGHT for +// that run — the cap is a load guard, and no data or index state +// depends on it. The victim's release() sees a foreign or missing file +// and leaves it alone. +// (b) Marker: between the token re-read and unlinkSync(marker) (ours or an +// orphan's), the marker could pass to another claimant, whose claim would +// then be removed. That only re-opens the slot to one more evictor, which +// still has to pass the slot identity check before deleting anything. +// Both need a stall of seconds landing on that exact syscall pair, and the +// only thing lost is one run's cap accounting, so they are accepted rather +// than traded for heavier machinery. A crash at any point orphans at most the +// marker, which the next contender breaks after it expires. +function evictStaleSlot(slotPath, inspected) { + const marker = `${slotPath}.evicting`; + breakOrphanedMarker(marker); + const token = `${process.pid}:${crypto.randomBytes(8).toString('hex')}`; + try { + fs.writeFileSync(marker, token, { flag: 'wx' }); + } catch { + return; // Another evictor holds this slot — leave it to that evictor. + } + try { + if ( + readMarkerToken(marker) === token && + sameSlotFile(fs.lstatSync(slotPath, { bigint: true }), inspected) + ) { + fs.unlinkSync(slotPath); + } + } catch { + /* slot already gone — the retry claims it */ + } finally { + if (readMarkerToken(marker) === token) { + try { + fs.unlinkSync(marker); + } catch { + /* already gone */ + } + } + } +} + function acquireHookSlot(gitNexusDir) { const lockDir = path.join(gitNexusDir, HOOK_LOCK_SUBDIR); try { @@ -28,6 +151,7 @@ function acquireHookSlot(gitNexusDir) { const release = () => { if (released) return; released = true; + process.removeListener('exit', release); try { // Only unlink if we still own the slot. If we appeared stale and // another hook took over, the file now belongs to it — leave alone. @@ -52,8 +176,10 @@ function acquireHookSlot(gitNexusDir) { } let isLive = false; let mtimeMs = Date.now(); + let inspected = null; try { - mtimeMs = fs.fstatSync(fd).mtimeMs; + inspected = fs.fstatSync(fd, { bigint: true }); + mtimeMs = Number(inspected.mtimeMs); const buf = Buffer.alloc(32); const n = fs.readSync(fd, buf, 0, 32, 0); const ownerStr = buf.slice(0, n).toString('utf-8').trim(); @@ -98,11 +224,9 @@ function acquireHookSlot(gitNexusDir) { isLive = false; } if (isLive) break; // Try the next slot. - try { - fs.unlinkSync(slotPath); - } catch { - /* another hook beat us to it — retry will hit EEXIST */ - } + // No stat means we cannot prove which file we judged stale; leave it + // (the retry re-inspects it) rather than risk deleting a fresh lock. + if (inspected) evictStaleSlot(slotPath, inspected); // Loop and retry this slot. } } diff --git a/gitnexus-cursor-integration/hooks/registry-query.cjs b/gitnexus-cursor-integration/hooks/registry-query.cjs index f5b2a0576..0607602db 100644 --- a/gitnexus-cursor-integration/hooks/registry-query.cjs +++ b/gitnexus-cursor-integration/hooks/registry-query.cjs @@ -248,40 +248,35 @@ function storageSlotName(repoPath) { return `${basename}-${digest}`; } +// Definedness matches the CLI's storage-resolver.ts: any value other than +// undefined (including '') counts as configured. function envOverridesStorage() { - const envPath = process.env[STORAGE_PATH_ENV]; - const envRoot = process.env[STORAGE_ROOT_ENV]; - return ( - (typeof envPath === 'string' && envPath.length > 0) || - (typeof envRoot === 'string' && envRoot.length > 0) - ); + return process.env[STORAGE_PATH_ENV] !== undefined || process.env[STORAGE_ROOT_ENV] !== undefined; } +// A set-but-invalid override (empty, relative, or containing NUL) makes +// storage unresolvable (the CLI's storage-resolver.ts throws); never fall +// back to the registry row. A filesystem root is invalid only for +// GITNEXUS_STORAGE_PATH (validateConfiguredStoragePath rejects it); +// GITNEXUS_STORAGE_ROOT accepts a filesystem root — storagePathFromRoot +// resolves the slot directly under it. function resolveEntryStoragePath(entry) { const envPath = process.env[STORAGE_PATH_ENV]; - if ( - typeof envPath === 'string' && - envPath.length > 0 && - !envPath.includes('\0') && - path.isAbsolute(envPath) - ) { + if (envPath !== undefined) { + if (!envPath || envPath.includes('\0') || !path.isAbsolute(envPath)) return null; const resolved = path.resolve(envPath); - if (path.isAbsolute(resolved)) return resolved; + // validateConfiguredStoragePath rejects a filesystem root. + return path.basename(resolved) ? resolved : null; } const envRoot = process.env[STORAGE_ROOT_ENV]; - if ( - typeof envRoot === 'string' && - envRoot.length > 0 && - !envRoot.includes('\0') && - path.isAbsolute(envRoot) - ) { + if (envRoot !== undefined) { + if (!envRoot || envRoot.includes('\0') || !path.isAbsolute(envRoot)) return null; const root = path.resolve(envRoot); const slot = storageSlotName(entry.path); - if (slot) { - const storagePath = path.join(root, slot); - if (samePath(path.dirname(storagePath), root)) return storagePath; - } + if (!slot) return null; + const storagePath = path.join(root, slot); + return samePath(path.dirname(storagePath), root) ? storagePath : null; } if (entry.storagePath !== undefined) { diff --git a/gitnexus-factory-plugin/.factory-plugin/plugin.json b/gitnexus-factory-plugin/.factory-plugin/plugin.json new file mode 100644 index 000000000..6195fe2b4 --- /dev/null +++ b/gitnexus-factory-plugin/.factory-plugin/plugin.json @@ -0,0 +1,11 @@ +{ + "name": "gitnexus", + "description": "Code intelligence powered by a knowledge graph. Provides execution flow tracing, blast radius analysis, and augmented search across your codebase.", + "version": "1.6.12", + "author": { + "name": "GitNexus" + }, + "homepage": "https://github.com/abhigyanpatwari/GitNexus", + "repository": "https://github.com/abhigyanpatwari/GitNexus", + "keywords": ["code-intelligence", "knowledge-graph", "mcp", "static-analysis"] +} diff --git a/gitnexus-factory-plugin/hooks/gitnexus-hook.js b/gitnexus-factory-plugin/hooks/gitnexus-hook.js new file mode 100644 index 000000000..8b631e4bb --- /dev/null +++ b/gitnexus-factory-plugin/hooks/gitnexus-hook.js @@ -0,0 +1,510 @@ +#!/usr/bin/env node +/** + * GitNexus Factory AI (Droid) plugin hook. + * + * PostToolUse — augments Grep/Glob/Execute searches with graph context and + * returns it via hookSpecificOutput.additionalContext. + * + * Reuses the Claude adapter's guards, bundled byte-identical: acquireHookSlot + * caps concurrent augment children per repo (#1486), and the LadybugDB owner + * probe skips the CLI augment when an MCP/serve process already holds the + * single-writer lock (#2396). The repo and its index storage are resolved via + * the same bundled registry lookup (registry-query.cjs), so external and + * branch-slot indexes work (#3060). On Unix the augment child runs under the + * probe's self-tested coreutils `timeout` guard, as in the Claude adapter + * (#2163), so a hook the runner kills cannot strand the CLI (see runAugment). + */ + +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const { acquireHookSlot } = require('./hook-lock.js'); +const { + hasGitNexusDbLockedByGitNexusServer, + resolveUnixGuardTimeout, +} = require('./hook-db-lock-probe.cjs'); +const { resolveHookRepo } = require('./registry-query.cjs'); + +// Pin the CLI instead of tracking `latest`: npm versions are immutable, so only +// a plugin revision can change what the fallback below executes. The release +// stamps this manifest (gitnexus/scripts/sync-plugin-manifests.mjs). +const { version: PINNED_VERSION } = require('../.factory-plugin/plugin.json'); + +function readInput() { + try { + return JSON.parse(fs.readFileSync(0, 'utf-8')); + } catch { + return {}; + } +} + +/** + * Split a command the way a POSIX shell would, so quoted and backslash-escaped + * patterns survive as one token. Kept identical to the Cursor adapter's + * tokenizer (#2938) so the two can collapse into a shared module later. + */ +function tokenizeShellWords(command) { + const tokens = []; + let current = ''; + let quote = null; + let escaped = false; + let hasToken = false; + + for (let index = 0; index < command.length; index += 1) { + const char = command[index]; + if (escaped) { + current += char; + escaped = false; + hasToken = true; + continue; + } + + if (quote === "'") { + if (char === "'") quote = null; + else current += char; + hasToken = true; + continue; + } + + if (quote === '"') { + if (char === '"') { + quote = null; + } else if (char === '\\') { + const next = command[index + 1]; + if (next === '$' || next === '`' || next === '"' || next === '\\') { + escaped = true; + } else { + current += '\\'; + } + } else { + current += char; + } + hasToken = true; + continue; + } + + if (char === '\\') { + const next = command[index + 1]; + if (next === undefined || /\s/.test(next) || next === "'" || next === '"' || next === '\\') { + escaped = true; + } else { + current += '\\' + next; + index += 1; + } + hasToken = true; + } else if (char === "'" || char === '"') { + quote = char; + hasToken = true; + } else if (/\s/.test(char)) { + if (hasToken) tokens.push(current); + current = ''; + hasToken = false; + } else if (char === ';' || char === '|' || char === '&') { + if (hasToken) tokens.push(current); + current = ''; + hasToken = false; + const next = command[index + 1]; + if ((char === '|' || char === '&') && next === char) { + tokens.push(char + char); + index += 1; + } else { + tokens.push(char); + } + } else { + current += char; + hasToken = true; + } + } + + if (escaped) current += '\\'; + if (hasToken) tokens.push(current); + return tokens; +} + +/** Recover the search pattern from an `rg`/`grep` command line. */ +function parseRgGrepPattern(cmd) { + const tokens = tokenizeShellWords(cmd); + let foundCmd = false; + let skipNext = false; + let skipNextAsPattern = false; + let endOfOptions = false; + let explicitPatternSeen = false; + let patternFileSeen = false; + const flagsWithValues = new Set([ + '-e', + '-f', + '--file', + '-m', + '--max-count', + '-A', + '-B', + '-C', + '-g', + '--glob', + '--iglob', + '-t', + '--type', + '--include', + '--exclude', + '--encoding', + '--path', + ]); + const rgValueFlags = new Set(['-r', '--replace']); + const patternFlags = new Set(['-e', '--regexp']); + const connectors = new Set(['&&', '||', ';', '|', '&']); + const wrappers = new Set([ + 'npx', + 'bunx', + 'pnpm', + 'yarn', + 'npm', + 'sudo', + 'env', + 'command', + 'time', + 'nice', + 'xargs', + 'dlx', + 'exec', + 'run', + 'git', + ]); + const wrapperFlagsWithValues = new Set([ + '--package', + '-p', + '--call', + '--prefix', + '--shell', + '--filter', + '--workspace', + '--dir', + '--cwd', + ]); + const basename = (token) => + token + .split(/[\\/]/) + .pop() + ?.replace(/\.(exe|cmd|bat)$/i, ''); + + let previousToken; + let seenWrapper = false; + let searchCommand = null; + for (const token of tokens) { + if (skipNext) { + skipNext = false; + if (skipNextAsPattern) { + skipNextAsPattern = false; + if (token.length >= 3) return token; + } + previousToken = token; + continue; + } + if (!foundCmd) { + if (connectors.has(token)) { + seenWrapper = false; + previousToken = token; + continue; + } + const commandName = basename(token); + if (wrappers.has(commandName)) { + seenWrapper = true; + previousToken = token; + continue; + } + if (seenWrapper && token.startsWith('-')) { + const flagName = token.split('=', 1)[0]; + if (!token.includes('=') && wrapperFlagsWithValues.has(flagName)) skipNext = true; + previousToken = token; + continue; + } + if (seenWrapper && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) { + previousToken = token; + continue; + } + const atCommandPosition = + previousToken === undefined || + connectors.has(previousToken) || + wrappers.has(basename(previousToken)) || + seenWrapper; + if (atCommandPosition && (commandName === 'rg' || commandName === 'grep')) { + foundCmd = true; + searchCommand = commandName; + } else if (seenWrapper) { + seenWrapper = false; + } + previousToken = token; + continue; + } + previousToken = token; + if (endOfOptions) { + if (explicitPatternSeen || patternFileSeen) continue; + return token.length >= 3 ? token : null; + } + if (token === '--') { + endOfOptions = true; + continue; + } + if (token.startsWith('-')) { + if (token === '-f' || token === '--file') { + skipNext = true; + patternFileSeen = true; + continue; + } + if (token.startsWith('--file=')) { + patternFileSeen = true; + continue; + } + if (token.startsWith('--regexp=')) { + explicitPatternSeen = true; + const value = token.slice('--regexp='.length); + if (value.length >= 3) return value; + continue; + } + const attachedPattern = token.match(/^-e(.+)$/); + if (attachedPattern) { + explicitPatternSeen = true; + if (attachedPattern[1].length >= 3) return attachedPattern[1]; + continue; + } + if ( + flagsWithValues.has(token) || + patternFlags.has(token) || + (searchCommand === 'rg' && rgValueFlags.has(token)) + ) { + skipNext = true; + skipNextAsPattern = patternFlags.has(token); + if (skipNextAsPattern) explicitPatternSeen = true; + } + continue; + } + if (explicitPatternSeen || patternFileSeen) continue; + return token.length >= 3 ? token : null; + } + return null; +} + +/** Factory's shell tool is `Execute` (Claude's is `Bash`); Grep/Glob match Claude's. */ +function extractPattern(toolName, toolInput) { + if (toolName === 'Grep') { + return toolInput.pattern || null; + } + + if (toolName === 'Glob') { + const raw = toolInput.pattern || ''; + const match = raw.match(/[*\/]([a-zA-Z][a-zA-Z0-9_-]{2,})/); + return match ? match[1] : null; + } + + if (toolName === 'Execute') { + const cmd = toolInput.command || ''; + if (!/\brg\b|\bgrep\b/.test(cmd)) return null; + return parseRgGrepPattern(cmd); + } + + return null; +} + +/** + * 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'; +} + +/** + * Keep only the augment block: stderr from the first `[GitNexus]` marker on, or + * '' when there is none, so npm/Node/LadybugDB warnings never reach the agent. + * Kept identical to the Claude adapter's copy so the two can be shared later. + */ +function extractAugmentContext(stderr) { + const output = (stderr || '').trim(); + const marker = output.indexOf('[GitNexus]'); + 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 + // warnings, parser errors, etc. — remain recoverable on the hook's own + // stderr. The untruncated payload lets operators see exactly what was + // filtered out instead of a 180-char JSON-quoted preview. + const discarded = marker === -1 ? output : output.slice(0, marker).trim(); + if (discarded.length > 0) { + process.stderr.write(`[GitNexus hook] augment stderr discarded prefix:\n${discarded}\n`); + } + } + return marker === -1 ? '' : output.slice(marker).trim(); +} + +/** + * Absolute path of a runnable (regular file, X_OK) `command` on PATH, or null. + * POSIX-only: used where the timeout guard would otherwise mask a missing + * launcher as the guard's own exit 127 instead of a spawn ENOENT. + */ +function findOnPath(command) { + for (const dir of (process.env.PATH || '').split(path.delimiter).filter(Boolean)) { + const candidate = path.join(dir, command); + try { + if (!fs.statSync(candidate).isFile()) continue; + fs.accessSync(candidate, fs.constants.X_OK); + return candidate; + } catch { + /* not a runnable file here */ + } + } + return null; +} + +/** + * Run `gitnexus augment` for `pattern` and return its `[GitNexus]` block — the + * augment CLI writes results to stderr because LadybugDB's native module + * captures stdout at the OS fd level. Launcher noise is filtered out by + * extractAugmentContext, so noise-only stderr yields ''. + * + * GITNEXUS_HOOK_CLI_PATH is tried first and run as `node `, the only form + * that works on Windows, where Node refuses to spawn the `.cmd` shims without a + * shell (CVE-2024-27980). Otherwise a PATH binary, and a version-pinned npx + * only when no PATH binary exists. Exactly one tier runs, so a no-match search + * (exit 0, empty stderr) or a timeout never spends a second 8s budget on npx + * past the 10s hook timeout in hooks.json. + * + * Orphan guard (#2163, ported from the Claude adapter's runGitNexusCli): on + * Unix every tier runs under the probe's self-tested coreutils `timeout`, so a + * hook killed by the runner cannot strand the CLI. The direct tiers (the CLI is + * the guard's child) use `-k 1` TERM-first; npx (guard → npx → CLI grandchild) + * uses `-s KILL`, which group-kills at budget — TERM-first would only kill the + * obedient npx parent and let `timeout` exit before its `-k` escalation, leaving + * a SIGTERM-immune CLI running. Residual gaps are the Claude adapter's: a + * busybox guard signals only its direct child, and when the hook itself is + * alive the inner spawnSync timeout SIGTERMs the guard, which forwards TERM, not + * KILL, to the npx group. Because the guard reports a missing command as its + * own exit 127 rather than ENOENT, the guarded PATH tier decides presence with + * findOnPath first. Windows (no coreutils; the self-test spawns /bin/sh) and an + * unresolved guard (e.g. macOS without Homebrew coreutils, or + * GITNEXUS_HOOK_TIMEOUT_PATH=disabled) keep the plain spawn and the ENOENT + * fallthrough. + * + * SECURITY: `pattern` follows the `--` end-of-options marker and never reaches a + * shell (the Windows fallback invokes `npx.cmd` directly rather than + * `shell: true`), so `-rf` or `$(...)` is inert. + */ +function runAugment(pattern, cwd) { + const isWin = process.platform === 'win32'; + const args = ['augment', '--', pattern]; + const timeoutMs = 8000; + const spawnOpts = { + encoding: 'utf-8', + timeout: timeoutMs, + cwd, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + }; + // An older bundled probe without the export degrades to the unwrapped spawn. + const guard = + isWin || typeof resolveUnixGuardTimeout !== 'function' ? null : resolveUnixGuardTimeout(); + if (!isWin && !guard && isDebugEnabled()) { + process.stderr.write( + '[GitNexus hook] no usable timeout/gtimeout guard; augment CLI child runs unguarded\n', + ); + } + const guardSecs = String(Math.ceil(timeoutMs / 1000) + 1); + // Only a clean exit 0 yields context; a spawn error, throw or non-zero exit is ''. + // `groupKill` selects the npx arm's `-s KILL` (see the docblock). + const spawnAugment = (cmd, argv, groupKill = false) => { + const [file, fileArgs] = guard + ? [guard, [...(groupKill ? ['-s', 'KILL'] : []), '-k', '1', guardSecs, cmd, ...argv]] + : [cmd, argv]; + try { + const child = spawnSync(file, fileArgs, spawnOpts); + if (!child.error && child.status === 0) return extractAugmentContext(child.stderr); + } catch { + /* graceful failure */ + } + return ''; + }; + + const hookCli = process.env.GITNEXUS_HOOK_CLI_PATH; + if (hookCli && String(hookCli).trim() && fs.existsSync(String(hookCli))) { + return spawnAugment(process.execPath, [String(hookCli), ...args]); + } + + if (guard) { + // Guarded (Unix): only a missing launcher falls through to npx. + const launcher = findOnPath('gitnexus'); + if (launcher) return spawnAugment(launcher, args); + } else { + // Only ENOENT (no launcher on PATH) falls through to npx. Windows EINVAL for + // `gitnexus.cmd` does not: `npx.cmd` would fail the same way without a shell. + try { + const child = spawnSync(isWin ? 'gitnexus.cmd' : 'gitnexus', args, spawnOpts); + if (!child.error || child.error.code !== 'ENOENT') { + return !child.error && child.status === 0 ? extractAugmentContext(child.stderr) : ''; + } + } catch (err) { + if (!err || err.code !== 'ENOENT') return ''; + } + } + + return spawnAugment( + isWin ? 'npx.cmd' : 'npx', + ['-y', `gitnexus@${PINNED_VERSION}`, ...args], + true, + ); +} + +function main() { + try { + const input = readInput(); + if ((input.hook_event_name || '') !== 'PostToolUse') return; + + const cwd = input.cwd || process.cwd(); + if (!path.isAbsolute(cwd)) return; + + const toolName = input.tool_name || ''; + if (toolName !== 'Grep' && toolName !== 'Glob' && toolName !== 'Execute') return; + + const pattern = extractPattern(toolName, input.tool_input || {}); + if (!pattern || pattern.length < 3) return; + + // Registry row first (persisted external storagePath wins); a local owned + // `.gitnexus` is the fallback — same lookup as the Claude/Cursor hooks. + const repo = resolveHookRepo(cwd); + if (!repo) return; + + const release = acquireHookSlot(repo.storagePath); + if (!release) return; // all per-repo augment slots held by concurrent sessions + + let result = ''; + try { + if (hasGitNexusDbLockedByGitNexusServer(repo.lbugPath, process.pid)) { + // #2396: an MCP/serve process owns the single-writer DB, so a competing + // CLI augment would only contend on the lock. Its MCP tools cover + // augmentation instead — skip silently. + return; + } + result = runAugment(pattern, cwd); + } catch { + /* graceful failure */ + } finally { + release(); + } + + if (result && result.trim()) { + console.log( + JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'PostToolUse', + additionalContext: result.trim(), + }, + }), + ); + } + } catch { + /* never let the hook break the tool call */ + } +} + +if (require.main === module) main(); + +module.exports = { parseRgGrepPattern, tokenizeShellWords }; diff --git a/gitnexus-factory-plugin/hooks/hook-db-lock-probe.cjs b/gitnexus-factory-plugin/hooks/hook-db-lock-probe.cjs new file mode 100644 index 000000000..3e98f5901 --- /dev/null +++ b/gitnexus-factory-plugin/hooks/hook-db-lock-probe.cjs @@ -0,0 +1,759 @@ +/** + * Cross-platform best-effort probe: does another process hold dbPath open + * with a command line that looks like a GitNexus MCP/serve server? + * + * Backends (no user-installed Sysinternals): + * - Linux: cmdline-first procfs scan under /proc, no lsof at all (#2180). Three + * phases, cheapest first: (0) read /proc//comm — a tiny task->comm read + * that never touches the target's mm — and keep only PIDs whose comm is a + * plausible node/gitnexus server; (1) read up to GITNEXUS_HOOK_PROC_CMDLINE_MAX + * bytes of /proc//cmdline via openSync+readSync (bounded, so a D-state + * holder stuck on mmap_lock or a giant argv can't wedge the hook) and prefilter + * with isGitNexusServerCommand; (2) only for the 0..N survivors, stat their + * /proc//fd/* and compare dev+inode against the target lbug. The lbug + * handle is fd-visible (a @ladybugdb/core property), so this finds every real + * owner without scanning every fd of every process. + * - macOS / *BSD / etc.: trusted lsof + ps (absolute paths first). + * - Windows: Restart Manager (rstrtmgr) via bundled PowerShell script + + * Win32_Process for command lines; trusted powershell.exe under %SystemRoot%. + * + * Fail matrix: + * - Linux proc scan: owner found -> fail-closed (skip augment); budget exhausted + * (GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS) -> fail-CLOSED (#2180). This is a + * deliberate change from the old "timeout -> fail-open then try lsof" path. + * End-to-end the busy-host outcome is unchanged: the old code's lsof fallback + * ETIMEDOUT'd on the very hosts where the scan ran out of budget and ALSO + * failed closed there — the lsof leg only ever added 1-2s of dead work plus + * the orphan-storm risk it caused (#2163). What changes is that an overloaded + * host now self-throttles immediately (the throttle the incident needed) + * instead of paying for a doomed lsof. Mid-load hosts that used to fall + * through to a successful lsof now answer from the scan directly (faster) or, + * if even the scan can't finish in budget, fail closed (self-throttle) — a + * bounded, documented tradeoff, never an orphan. + * - macOS / other Unix: fail-open on most errors; fail-closed only on lsof + * ETIMEDOUT, matching the hook contract. + * - Windows: fail-closed only on PowerShell ETIMEDOUT. + * + * Unix subprocess containment contract (#2163): + * - lsof/ps are wrapped in coreutils `timeout`/`gtimeout` when a working + * wrapper is found (`timeout -k 1 lsof ...`). If this hook process + * is itself SIGKILLed (e.g. by the runner's 10s hook timeout) the wrapper + * survives, SIGTERMs its child at the budget (2s lsof / 1s ps) and SIGKILLs + * it 1s later — orphan lifetime is bounded at ~3s instead of unbounded. + * - GITNEXUS_HOOK_TIMEOUT_PATH: the sentinel value `disabled` switches the + * wrapper off deterministically; any other value is adopted only when it + * exists AND passes a one-shot `-k` exit-propagation self-test — otherwise + * resolution FALLS THROUGH to the built-in candidate list (first self-test + * pass wins), so no malformed value of any shape can silently disable + * orphan containment. + * - The gitnexus server is lazy-open + sticky-hold: an idle MCP server holds + * ZERO lbug fds until the repo's first MCP query, then keeps the fd open. + * A probe before that first query is therefore always false — a known, + * pre-existing race, not a bug in this probe. + * - resolveUnixGuardTimeout is exported so the hook adapters can wrap the + * `gitnexus augment` CLI child — the longest-lived hook subprocess (7s + * local / 12s npx inner budgets) — in the same guard; see runGitNexusCli + * in the adapters (#2163 follow-up). + */ + +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +function isGitNexusServerCommand(command) { + const hasServerMode = /(?:^|\s)(mcp|serve)(?:\s|$)/.test(command); + const hasGitNexus = + /(?:^|[/\\\s])gitnexus(?:\.cmd)?(?:\s|$)/.test(command) || + /node_modules[/\\]gitnexus[/\\]/.test(command); + return hasServerMode && hasGitNexus; +} + +// GITNEXUS_DEBUG-gated stderr diagnostics. Reuses the exact gating predicate the +// Windows ps1-load warning already uses (===' 1' / ==='true') so there is one +// debug convention in this file, and writes via process.stderr.write (NOT a +// spawn) so it never perturbs the windowsHide spawn-count invariant. +function debugLog(msg) { + if (process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true') { + process.stderr.write(`[GitNexus hook] ${msg}\n`); + } +} + +function resolveHookBinary(tool) { + const envKey = tool === 'lsof' ? 'GITNEXUS_HOOK_LSOF_PATH' : 'GITNEXUS_HOOK_PS_PATH'; + // Trim once, exactly as hasMissingHookBinaryOverride does, so a padded but + // valid override (" /tmp/lsof ") is both accepted there and used here. + const fromEnv = process.env[envKey] ? String(process.env[envKey]).trim() : ''; + if (fromEnv && fs.existsSync(fromEnv)) { + return fromEnv; + } + const candidates = + tool === 'lsof' + ? ['/usr/bin/lsof', '/usr/sbin/lsof', '/sbin/lsof', tool] + : ['/bin/ps', '/usr/bin/ps', tool]; + for (const candidate of candidates) { + if (candidate === tool) return tool; + try { + if (fs.existsSync(candidate)) return candidate; + } catch { + /* ignore */ + } + } + return tool; +} + +function hasMissingHookBinaryOverride(tool) { + const envKey = tool === 'lsof' ? 'GITNEXUS_HOOK_LSOF_PATH' : 'GITNEXUS_HOOK_PS_PATH'; + const fromEnv = process.env[envKey]; + if (!fromEnv || !String(fromEnv).trim()) return false; + try { + return !fs.existsSync(String(fromEnv).trim()); + } catch { + return true; + } +} + +// Sentinel: +// undefined = not resolved yet (resolve lazily, on first lsof/ps fallback) +// string = self-tested coreutils timeout/gtimeout path (use as wrapper) +// null = no usable wrapper (disabled, none found, or self-test failed) +let unixGuardTimeoutCache; + +/** + * Resolve a coreutils `timeout`/`gtimeout` binary to wrap lsof/ps with + * (#2163). Unix-only by contract: the probe's win32 dispatch returns before + * reaching it, and the exported callers (the adapters' runGitNexusCli, + * #2163 follow-up) must check the platform first — the self-test below + * spawns /bin/sh. The memoized result is module-wide, so probe and adapter + * share one lazy self-test per hook process. + * + * GITNEXUS_HOOK_TIMEOUT_PATH semantics: the sentinel `disabled` turns the + * wrapper off; any other value is only a CANDIDATE — an existing file path + * is tried first, but it must pass the `-k` exit-propagation self-test to + * be adopted. On any failure (non-existent path, directory, non-executable + * file, wrapper without `-k` support, always-exit-0 stub, …) resolution + * falls through to the built-in candidates below, tried in order, first + * self-test pass wins. This is strictly stronger than the sibling + * GITNEXUS_HOOK_LSOF_PATH / GITNEXUS_HOOK_PS_PATH overrides (which only + * check existence): no bad env value of ANY shape can silently disable + * orphan containment. + * + * Lazy self-test: candidates are probed only when the lsof/ps fallback is + * first reached, and the result is memoized. A candidate is adopted only + * when `timeout -k 1 1 /bin/sh -c 'exit 42'` exits 42 — i.e. it must RUN + * the wrapped command AND PROPAGATE its exit status. This rejects two + * failure shapes: wrappers without the coreutils `-k` flag — busybox <1.34, + * toybox, broken symlinks — which would exit with a usage error without + * ever running lsof, silently converting the lsof-ETIMEDOUT fail-closed + * contract into fail-open (#1492 regression); and always-exit-0 stubs + * (/bin/true shapes), which would otherwise be adopted and "succeed" every + * wrapped spawn instantly without running it — a constant no-owner probe + * answer and, worse, a silently dead augment (status 0, empty stderr passes + * the adapters' success check with no context; #2163 follow-up review). + * Only when EVERY candidate fails does the probe fall back to the unwrapped + * status quo (memoized null). busybox ≥1.34 passes the test and is fully + * usable for everything THIS file spawns (lsof/ps are the guard's direct + * children) and for the adapters' direct-exec arm. The adapters' npx arm + * additionally relies on coreutils' process-GROUP signalling for its + * `-s KILL` grandchild reaping; busybox signals only its direct child, and + * this self-test deliberately does not probe that capability — see the + * adapter docblocks for the residual-gap statement. + */ +function passesGuardSelfTest(guard) { + try { + const selfTest = spawnSync(guard, ['-k', '1', '1', '/bin/sh', '-c', 'exit 42'], { + encoding: 'utf-8', + timeout: 3000, + stdio: ['ignore', 'ignore', 'ignore'], + windowsHide: true, + }); + return !selfTest.error && selfTest.status === 42; + } catch { + return false; + } +} + +function resolveUnixGuardTimeout() { + if (unixGuardTimeoutCache !== undefined) return unixGuardTimeoutCache; + unixGuardTimeoutCache = null; + const fromEnv = process.env.GITNEXUS_HOOK_TIMEOUT_PATH; + const trimmed = fromEnv ? String(fromEnv).trim() : ''; + if (trimmed === 'disabled') return unixGuardTimeoutCache; + const candidates = []; + // Resolve the override against THIS process's cwd — the directory the + // existsSync check and the self-test run in — so the cached/returned path is + // always absolute. The adapters spawn the wrapper with a different `cwd` + // (the tool request's), where a relative value would resolve elsewhere + // (ENOENT), and a slashless name would switch to a PATH lookup. + const override = trimmed ? path.resolve(trimmed) : ''; + if (override && fs.existsSync(override)) candidates.push(override); + for (const builtin of [ + '/usr/bin/timeout', + '/bin/timeout', + '/opt/homebrew/bin/gtimeout', + '/usr/local/bin/gtimeout', + ]) { + try { + if (fs.existsSync(builtin)) candidates.push(builtin); + } catch { + /* ignore */ + } + } + for (const candidate of candidates) { + if (passesGuardSelfTest(candidate)) { + unixGuardTimeoutCache = candidate; + break; + } + } + return unixGuardTimeoutCache; +} + +function resolveWindowsPowerShellPath() { + const fromEnv = process.env.GITNEXUS_HOOK_POWERSHELL_PATH; + if (fromEnv && String(fromEnv).trim() && fs.existsSync(String(fromEnv).trim())) { + return String(fromEnv).trim(); + } + const root = process.env.SystemRoot || 'C:\\Windows'; + const ps = path.join(root, 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); + if (fs.existsSync(ps)) return ps; + const psWow = path.join(root, 'SysWOW64', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); + if (fs.existsSync(psWow)) return psWow; + return 'powershell.exe'; +} + +// Sentinel: +// undefined = not loaded yet (try the read) +// string = encoded PowerShell command (successful load) +// null = load attempted and failed (do not retry; warning already emitted) +let windowsRmListPsEncodedCommandCache; +let windowsRmListPsLoadFailureWarned = false; +function getWindowsRmListEncodedCommand() { + if (windowsRmListPsEncodedCommandCache !== undefined) { + return windowsRmListPsEncodedCommandCache; + } + try { + const ps1Path = path.join(__dirname, 'win-rm-list-json.ps1'); + const src = fs + .readFileSync(ps1Path, 'utf8') + .replace(/^\uFEFF/, '') + .replace(/\r\n/g, '\n'); + windowsRmListPsEncodedCommandCache = Buffer.from(src, 'utf16le').toString('base64'); + } catch (err) { + windowsRmListPsEncodedCommandCache = null; + if ( + !windowsRmListPsLoadFailureWarned && + (process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true') + ) { + windowsRmListPsLoadFailureWarned = true; + const msg = err && err.message ? String(err.message).slice(0, 200) : 'unknown'; + process.stderr.write(`[GitNexus hook] win-rm-list-json.ps1 load failed: ${msg}\n`); + } + } + return windowsRmListPsEncodedCommandCache; +} + +function hasGitNexusServerOwnerWindows(dbPathAbs, myPid) { + const encoded = getWindowsRmListEncodedCommand(); + if (!encoded) return false; + const psExe = resolveWindowsPowerShellPath(); + const r = spawnSync( + psExe, + [ + '-NoProfile', + '-NonInteractive', + '-ExecutionPolicy', + 'Bypass', + '-STA', + '-EncodedCommand', + encoded, + ], + { + encoding: 'utf-8', + timeout: 6000, + stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, + env: { ...process.env, GITNEXUS_HOOK_RM_TARGET: dbPathAbs }, + }, + ); + // ETIMEDOUT means the PowerShell probe didn't return in time; treat as 'unresponsive process holds DB' → fail-closed (skip augment). + if (r.error) return r.error.code === 'ETIMEDOUT'; + if (r.status !== 0) return false; + let rows; + try { + rows = JSON.parse(String(r.stdout || '').trim() || '[]'); + } catch { + return false; + } + if (!Array.isArray(rows)) return false; + for (const row of rows) { + const procId = Number(row.pid); + const cmd = String(row.cmd || ''); + if (!Number.isFinite(procId) || procId === myPid) continue; + if (isGitNexusServerCommand(cmd)) return true; + } + return false; +} + +// The procfs root every Linux scan path reads from. Production is always /proc; +// GITNEXUS_HOOK_PROC_ROOT only exists so unit tests can inject a fixture tree +// (comm + cmdline + fd symlinks) and assert the three-phase logic without +// scanning the real, ~hundreds-of-process /proc of the test host. +// +// Test-only gate (F4): the override is honored ONLY under a test runner — +// vitest injects VITEST="true" and NODE_ENV="test" into every worker (verified; +// a production hook is `node .cjs` with neither set). Without the gate, a +// production env that accidentally leaked GITNEXUS_HOOK_PROC_ROOT (pointing at an +// empty/bad tree) would make readdirSync find no pids -> 'not-owned' -> Linux +// owner detection silently OFF (fail-OPEN: augment races the real server for the +// lbug, the #1492 class). Gating to the test signal makes that leak inert in +// production (always /proc) while the fake-procfs unit tests, which run under +// vitest, still inject freely. Unset env (or non-test context) => /proc, so the +// production path is byte-for-byte the historical behavior. +function isTestContext() { + return ( + process.env.VITEST === 'true' || process.env.VITEST === '1' || process.env.NODE_ENV === 'test' + ); +} +function getProcRoot() { + if (!isTestContext()) return '/proc'; + const raw = process.env.GITNEXUS_HOOK_PROC_ROOT; + return raw && String(raw).trim() ? String(raw) : '/proc'; +} + +// Max bytes read from /proc//cmdline in Phase 1. Bounded by default so a +// D-state holder wedged on mmap_lock, or a process with a pathological multi-MB +// argv, can't stall the hook. 16 KiB comfortably clears a realistic +// `node mcp` line +// (the `mcp`/`serve` mode token lives at the very tail, so the cap must be large +// enough to reach it — see PROC_CMDLINE_FLOOR escalation below). Overridable for +// tests via GITNEXUS_HOOK_PROC_CMDLINE_MAX: an integer in +// [PROC_CMDLINE_FLOOR, PROC_CMDLINE_CEIL] is used as-is; a larger integer is +// CLAMPED to PROC_CMDLINE_CEIL; anything else (below the floor, fractional, +// non-numeric, Infinity) falls back to the 16 KiB default. +const PROC_CMDLINE_FLOOR = 4096; +// Upper bound for a single cmdline read chunk, and the absolute ceiling of the +// escalation path in readLinuxCmdline (same 256 KiB — no single read may exceed +// what the whole escalation is allowed to collect). Without it, an oversized +// override (e.g. 2**40 — past buffer.constants.MAX_LENGTH on older Node lines +// and unallocatable in practice on any) made Buffer.allocUnsafe throw; readLinuxCmdline's catch turned that into '' (a +// NON-candidate), so a real server owner was silently missed (fail-OPEN, the +// #1492 race). Oversized values are clamped rather than defaulted: the operator +// asked for MORE bytes, and the ceiling is the most the read will ever collect +// anyway, so clamping honours the intent while keeping allocation bounded. +const PROC_CMDLINE_CEIL = 262144; +function getCmdlineMaxBytes() { + const raw = process.env.GITNEXUS_HOOK_PROC_CMDLINE_MAX; + // Number() (not parseInt) so "8e3" reads as 8000, not 8 (parseInt stops at + // 'e'). The `raw && String(raw).trim()` guard keeps empty/whitespace on the + // default; trailing garbage ("8abc") now -> NaN -> default (stricter). + const n = raw && String(raw).trim() ? Number(String(raw).trim()) : NaN; + // Number.isInteger rejects NaN, +/-Infinity and fractions (a fractional + // Buffer/readSync length is not a byte count). + if (Number.isInteger(n) && n >= PROC_CMDLINE_FLOOR) return Math.min(n, PROC_CMDLINE_CEIL); + return 16384; +} + +// Phase 0 comm prefilter. /proc//comm is the kernel task->comm string, +// capped at 16 bytes INCLUDING the trailing NUL — i.e. at most 15 visible +// chars, truncated by the kernel with no marker. So a process whose real name +// is longer than 15 chars shows a 15-char prefix here. The match below is +// therefore truncation-safe in BOTH directions (a whitelist name that is a +// prefix of comm, or comm that is a prefix of a whitelist name, both count) to +// guarantee we never drop a real owner at this cheap stage — Phase 2's dev+ino +// fd check is the real authority; Phase 0/1 only exist to skip the overwhelming +// majority (kernel threads, shells, editors) cheaply. +// +// The whitelist is calibrated against what a real `gitnexus mcp`/`serve` server +// actually reports for comm. Observed on production hosts: the server renames +// its main thread, so comm reads `MainThread` (via @ladybugdb/core's +// worker_threads setup), NOT `node` — omitting it would blind the probe to +// every real server (#1492-class owner miss). We also keep the plausible +// launcher/runtime basenames in case a future build does not rename the thread. +// Conservative by design: over-collecting a few extra candidates only costs a +// bounded number of Phase 1 cmdline reads. +const COMM_CANDIDATES = ['node', 'gitnexus', 'bun', 'deno', 'npm', 'npx', 'MainThread']; +function commLooksLikeServer(comm) { + const c = comm.trim(); + if (!c) return false; + for (const name of COMM_CANDIDATES) { + if (name === c || name.startsWith(c) || c.startsWith(name)) return true; + } + return false; +} + +function readProcComm(procRoot, pidStr) { + try { + return fs + .readFileSync(path.join(procRoot, pidStr, 'comm'), 'utf8') + .replace(/\0+/g, '') + .trim(); + } catch { + return ''; + } +} + +// Timeout sentinel for readLinuxCmdline (F3). MUST be distinct from the +// "unreadable/empty" return value (''): '' flows through isGitNexusServerCommand +// as a NON-candidate (both regexes are false on ''), so the Phase 1 caller +// `continue`s past it — correct for a raced/openSync-failed pid, but a FAIL-OPEN +// bug if it ever meant "I ran out of budget mid-read" (a real owner whose +// escalation timed out would be silently dropped, racing the lbug -> #1492). A +// unique Symbol can never collide with any cmdline string, so the caller can +// branch on it explicitly and map a mid-read timeout to the tri-state 'timeout' +// (fail-CLOSED) instead of swallowing it as a non-candidate. +const CMDLINE_TIMEOUT = Symbol('gitnexus.cmdline.timeout'); + +// Bounded /proc//cmdline read for Phase 1. openSync+readSync (not +// readFileSync) so a D-state holder cannot stall the hook on a huge or +// never-EOF argv: we read in `cap`-sized chunks and stop as soon as the text +// holds both server tokens, at EOF, at PROC_CMDLINE_CEIL, or when the scan +// budget runs out (see below). cmdline separates argv with NULs; convert to +// spaces for isGitNexusServerCommand. +// +// Owner-miss guard for the 4 KB cap: the `gitnexus` token usually sits in the +// first path component while the `mcp`/`serve` mode token is the LAST argv, so +// a naive 4 KB read could clip the mode token off a server launched with a very +// long interpreter path and silently miss a real owner. We mitigate two ways: +// (a) the default cap (16 KiB) already clears realistic lines, so almost every +// process is decided by the first read hitting EOF; (b) a read that fills the +// cap stops early ONLY once it holds BOTH tokens (decided owner). Holding one +// token, or neither, decides nothing: interpreter flags can put a mode-looking +// word first (`node --require mcp .../gitnexus/... serve`) or push the gitnexus +// path past the first chunk. So we keep reading in cap-sized chunks until both +// tokens appear, the file ends, or the hard ceiling is reached. Only processes +// that passed the Phase 0 comm prefilter AND have a cmdline longer than the cap +// ever escalate, and each escalation step is budget-gated (below). +// +// Budget (F3): the escalation loop above is the one place a SINGLE pathological +// candidate could read up to HARD_CEIL (256 KiB) before the next scan-level +// budget check, weakening the timeout contract. `outOfBudget` (the scan's shared +// deadline callback) is checked once per escalation iteration; on expiry we +// return CMDLINE_TIMEOUT (NOT '') so the caller can fail-closed honestly rather +// than mistake the partial read for a non-candidate. Reads that simply can't +// open / error out still return '' (genuinely "not a readable candidate"). +function readLinuxCmdline(procRoot, pidStr, cap, outOfBudget) { + const file = path.join(procRoot, pidStr, 'cmdline'); + let fd; + try { + fd = fs.openSync(file, 'r'); + } catch { + return ''; + } + try { + const HARD_CEIL = PROC_CMDLINE_CEIL; // 256 KiB absolute ceiling for the escalation path + let collected = Buffer.alloc(0); + let offset = 0; + let chunkCap = cap; + for (;;) { + // allocUnsafe is safe here: readSync fills exactly [0, bytes), only + // buf.subarray(0, bytes) is consumed, and Buffer.concat deep-copies that + // slice into `collected`, so the uninitialized tail never reaches decode. + const buf = Buffer.allocUnsafe(chunkCap); + const bytes = fs.readSync(fd, buf, 0, chunkCap, offset); + if (bytes <= 0) break; + collected = Buffer.concat([collected, buf.subarray(0, bytes)]); + offset += bytes; + const text = collected.toString('utf8').replace(/\0+/g, ' '); + // Stop early only when the partial read is DECIDED: both the gitnexus + // token and a mode token are present (isGitNexusServerCommand is exactly + // that conjunction). A partial read missing either token is undecided — + // the missing one may lie past the chunk boundary — so it keeps reading. + if (isGitNexusServerCommand(text)) break; // decided (positive) + if (bytes < chunkCap) break; // EOF: full cmdline read, definitive + if (offset >= HARD_CEIL) break; // bounded escalation only + // Budget gate the escalation: a single huge-argv candidate must not burn + // the whole scan deadline before we re-check. Return the timeout sentinel + // (never '') so the caller fails closed instead of treating us as a + // non-candidate. The sole caller (linuxProcScanFindGitNexusServer) always + // passes outOfBudget, so no presence guard is needed. + if (outOfBudget()) return CMDLINE_TIMEOUT; + chunkCap = cap; // keep reading more in cap-sized chunks + } + return collected.toString('utf8').replace(/\0+/g, ' ').trim(); + } catch { + return ''; + } finally { + try { + fs.closeSync(fd); + } catch { + /* ignore */ + } + } +} + +function resolveLinuxProcBudgetMs() { + const raw = process.env.GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS; + // Gate on the STRING's emptiness, NOT the parsed number's truthiness — the + // old `Number(raw && trim()) ? ... : 1200` form treated "0" as falsy and + // silently fell back to 1200 (#2180). Use Number() (not parseInt) so "16e3" + // reads as 16000, not 16 (parseInt stops at 'e'). The `&& String(raw).trim()` + // guard is load-bearing: without it a set-but-empty/whitespace value would be + // `Number("")===0` => budget 0 => immediate fail-CLOSED timeout (augment + // permanently skipped). With it, ''/whitespace => NaN => 1200 default, while a + // finite "0" still parses to an explicit, deterministic "no budget" => + // immediate timeout. Non-numeric / unset => default 1200. + const n = raw != null && String(raw).trim() ? Number(String(raw).trim()) : NaN; + if (!Number.isFinite(n)) return 1200; + return n; // may be <= 0, meaning "out of budget on the first check" +} + +// Returns one of: 'owned' (a non-self process with a GitNexus-server cmdline +// holds the target lbug fd), 'not-owned' (scan completed, no such owner), or +// 'timeout' (the per-scan budget was exhausted before a verdict). The name is +// pinned by a source-contract test; only the return TYPE changed (#2180: +// boolean -> tri-state, so the dispatcher can fail-closed on 'timeout'). +function linuxProcScanFindGitNexusServer(dbPathAbs, myPid) { + const budget = resolveLinuxProcBudgetMs(); + // A non-positive budget is an explicit, deterministic "no time to scan" => + // immediate timeout (the #2180 test vector, and the only correct reading of + // the fixed parse: "0" must NOT mean 1200). Returning before any procfs read + // keeps it instantaneous regardless of host load. + if (budget <= 0) return 'timeout'; + const procRoot = getProcRoot(); + const cmdlineCap = getCmdlineMaxBytes(); + const start = Date.now(); + const outOfBudget = () => Date.now() - start > budget; + + let targetStat; + try { + targetStat = fs.statSync(dbPathAbs); + } catch { + // Caller already existsSync'd the path; a stat failure here is a transient + // race, treat as no owner (historical semantics). + return 'not-owned'; + } + + let procEntries; + try { + procEntries = fs.readdirSync(procRoot, { withFileTypes: true }); + } catch { + return 'not-owned'; + } + + // Phase 0 + Phase 1: collect the few PIDs whose comm AND cmdline look like a + // GitNexus server, without touching any fd yet. + const candidates = []; + for (const ent of procEntries) { + if (outOfBudget()) return 'timeout'; + if (!ent.isDirectory() || !/^\d+$/.test(ent.name)) continue; + const pid = Number.parseInt(ent.name, 10); + if (!Number.isFinite(pid) || pid === myPid) continue; + + // Phase 0: cheap comm prefilter. + const comm = readProcComm(procRoot, ent.name); + if (!comm) continue; // unreadable comm (kernel thread, raced exit) -> skip + if (!commLooksLikeServer(comm)) continue; + + // Phase 1: bounded cmdline read + isGitNexusServerCommand prefilter. + if (outOfBudget()) return 'timeout'; + const cmdline = readLinuxCmdline(procRoot, ent.name, cmdlineCap, outOfBudget); + // F3: a mid-read budget timeout returns the CMDLINE_TIMEOUT sentinel (a + // Symbol, never a string). Fail CLOSED on it rather than letting it fall + // through isGitNexusServerCommand as a non-candidate — a real owner whose + // escalation timed out must not be silently dropped (would fail-OPEN). + if (cmdline === CMDLINE_TIMEOUT) return 'timeout'; + if (!isGitNexusServerCommand(cmdline)) continue; + candidates.push(ent.name); + } + + // Phase 2: only now stat the fds of the (typically 0-2) survivors. + for (const pidStr of candidates) { + if (outOfBudget()) return 'timeout'; + const fdDir = path.join(procRoot, pidStr, 'fd'); + let fds; + try { + fds = fs.readdirSync(fdDir); + } catch (err) { + // F1: the old code returned 'owned' for EVERY non-ENOENT error. That was + // a correctness bug: /proc//fd is owner-only (mode 0500), so a + // cross-user/root `gitnexus mcp` serving a DIFFERENT repo passes Phase 0+1 + // (its cmdline matches) and then EACCES'es here — yet its dev+ino was + // NEVER compared against THIS lbug. Claiming 'owned' lets it permanently, + // silently suppress augment for a repo it does not actually lock. We now + // distinguish the failure shapes (all still fail-closed where we can't + // prove non-ownership, but 'timeout' is the HONEST verdict for + // "inconclusive", not the false-positive 'owned'): + const code = err && err.code; + if (code === 'ENOENT') { + // Process raced away between the candidate scan and now -> genuinely no + // longer an owner. Move on. + continue; + } + if (code === 'EACCES' || code === 'EPERM') { + // Permission-denied fd dir: cannot read fds, so ownership is + // UNVERIFIABLE. Fail closed honestly via 'timeout' (the dispatcher maps + // timeout -> true, same protective skip as before) WITHOUT lying that we + // confirmed ownership. Do NOT degrade to not-owned/fail-open: if this + // really is the owner, fail-open re-opens the #1492 lbug race; augment + // is optional context, so a conservative skip costs little. + debugLog( + `fd dir unreadable for candidate pid ${pidStr} (${code}); ownership ` + + `unverifiable, probe inconclusive -> fail-closed (timeout)`, + ); + return 'timeout'; + } + if (code === 'EIO' || code === 'ESTALE') { + // Genuine transient I/O against this candidate's fd dir — not evidence + // it does NOT hold the lbug. Treat as inconclusive and fail closed + // (timeout) rather than continue, so a real owner mid-I/O-blip is not + // dropped (would fail-open). + debugLog( + `fd dir transient I/O error for candidate pid ${pidStr} (${code}); ` + + `probe inconclusive -> fail-closed (timeout)`, + ); + return 'timeout'; + } + if (code === 'ENOTDIR') { + // The fd path is not a directory at all, so this is structurally not a + // real /proc//fd — not a plausible live owner. Move on. + debugLog( + `fd dir not a directory for candidate pid ${pidStr} (ENOTDIR); ` + + `treating candidate as non-owner -> continue`, + ); + continue; + } + // Any other error (EMFILE, ENFILE, ENOMEM, EINTR, no code, …) says nothing + // about whether this already-identified server candidate holds the lbug. + // Only ENOENT/ENOTDIR above establish non-ownership; everything else is + // inconclusive and fails closed via 'timeout', same as EACCES/EIO. + debugLog( + `fd dir read failed for candidate pid ${pidStr} ` + + `(${code || 'unknown'}); probe inconclusive -> fail-closed (timeout)`, + ); + return 'timeout'; + } + for (const fd of fds) { + if (outOfBudget()) return 'timeout'; + try { + const st = fs.statSync(path.join(fdDir, fd)); + if (st.dev === targetStat.dev && st.ino === targetStat.ino) { + return 'owned'; + } + } catch { + /* fd raced closed; ignore */ + } + } + } + + return 'not-owned'; +} + +function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { + const guard = resolveUnixGuardTimeout(); + // An explicit missing override models ENOENT and must fail open instead of + // falling through to a host binary with different process-table visibility. + if (hasMissingHookBinaryOverride('lsof')) return false; + const lsofPath = resolveHookBinary('lsof'); + // The spawnSync timeouts below (lsof 1000ms / ps 500ms) are deliberately + // SHORTER than the wrapper budgets (2s / 1s): on the supervised path Node's + // SIGTERM always fires first, so `error.code === 'ETIMEDOUT'` and the + // fail-closed contract are untouched. The wrapper only matters once this + // hook process has been SIGKILLed and can no longer deliver that SIGTERM. + const [lsofCmd, lsofArgs] = guard + ? [guard, ['-k', '1', '2', lsofPath, '-nP', '-t', '--', dbPathAbs]] + : [lsofPath, ['-nP', '-t', '--', dbPathAbs]]; + const lsof = spawnSync(lsofCmd, lsofArgs, { + encoding: 'utf-8', + timeout: 1000, + stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, + }); + if (lsof.error) return lsof.error.code === 'ETIMEDOUT'; + // Guard-mediated deaths map to "unresponsive holder" (fail-closed). Three + // result shapes, verified against coreutils 9.1: + // - signal-death: when `-k` escalates to SIGKILL, coreutils timeout + // SELF-RAISES the signal, so spawnSync reports {status: null, signal} + // with no .error (spawnSync's own ETIMEDOUT was handled above). The + // same shape appears when this hook is frozen >2s (SIGSTOP, laptop + // suspend) and the guard expires while it sleeps. By construction, a + // guard-wrapped probe that died by signal without spawnSync ETIMEDOUT + // is a budget/kill outcome. + // - 124: budget expired and the child exited after the plain SIGTERM. + // - 137: NOT the coreutils -k path — only exit-code-propagating wrappers, + // or a child SIGKILLed externally (e.g. the OOM killer). + if (guard && lsof.status === null && lsof.signal) return true; + if (guard && (lsof.status === 124 || lsof.status === 137)) return true; + + const pids = (lsof.stdout || '').split(/\s+/).filter(Boolean); + const psMissing = hasMissingHookBinaryOverride('ps'); + const psPath = resolveHookBinary('ps'); + for (const pid of pids) { + if (Number(pid) === myPid) continue; + // Missing ps means we cannot verify that this pid is a GitNexus server. + if (psMissing) continue; + const [psCmd, psArgs] = guard + ? [guard, ['-k', '1', '1', psPath, '-p', pid, '-o', 'command=']] + : [psPath, ['-p', pid, '-o', 'command=']]; + const ps = spawnSync(psCmd, psArgs, { + encoding: 'utf-8', + timeout: 500, + stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, + }); + if (ps.error) { + if (ps.error.code === 'ETIMEDOUT') return true; + continue; + } + // Same guard-mediated-death mapping as the lsof call above (signal-death + // from the -k escalation or a frozen hook; 124 budget expiry; 137 only + // for exit-code-propagating wrappers / external SIGKILL). + if (guard && ps.status === null && ps.signal) return true; + if (guard && (ps.status === 124 || ps.status === 137)) return true; + if (isGitNexusServerCommand(ps.stdout || '')) return true; + } + return false; +} + +/** + * @param {string} dbPath Absolute or relative path to the DB file (e.g. .../lbug). + * @param {number} myPid Current process PID (hook runner), excluded from matches. + */ +function hasGitNexusDbLockedByGitNexusServer(dbPath, myPid) { + if (!fs.existsSync(dbPath)) return false; + const dbPathAbs = path.resolve(dbPath); + + if (process.platform === 'win32') { + return hasGitNexusServerOwnerWindows(dbPathAbs, myPid); + } + + if (process.platform === 'linux') { + // #2180: cmdline-first procfs scan, no lsof. 'timeout' fails CLOSED + // (overloaded host self-throttles — the throttle the orphan-storm incident + // needed; the old lsof fallback ETIMEDOUT'd and failed closed on these same + // hosts anyway, only slower and with the orphan risk). 'not-owned' is the + // only false. See the fail matrix in the file header. + const verdict = linuxProcScanFindGitNexusServer(dbPathAbs, myPid); + return verdict !== 'not-owned'; + } + + return unixLsofPsFindGitNexusServer(dbPathAbs, myPid); +} + +module.exports = { + hasGitNexusDbLockedByGitNexusServer, + // Exported for white-box unit tests that must assert the tri-state verdict + // ('owned' | 'not-owned' | 'timeout') directly — the dispatcher collapses + // timeout and owned to the same boolean true, so the boolean API alone cannot + // distinguish the F1 EACCES->timeout fix from the old EACCES->owned bug. The + // Probe interface already declares this optional. Linux-only by contract; the + // name is pinned by a source-contract test. + linuxProcScanFindGitNexusServer, + // #2163 follow-up: the hook adapters wrap the augment CLI in the same + // guard. Returns a self-tested, always-ABSOLUTE wrapper path: the built-in + // candidates are absolute, and a GITNEXUS_HOOK_TIMEOUT_PATH override is + // path.resolve()d against this process's cwd before its existsSync check + // and self-test, so the adapters can spawn it under any `cwd` option. + // Returns null when the wrapper is disabled/unavailable. Never call on + // win32 (see its JSDoc). + resolveUnixGuardTimeout, + // Exported for white-box unit tests of the numeric-env parsing (#2183 review): + // Number()-not-parseInt so "16e3" reads as 16000, plus the empty/whitespace + // guard that keeps a set-but-empty budget on the 1200 default instead of an + // immediate fail-closed timeout. Tested directly because the values are + // otherwise only observable indirectly through scan timing/escalation. + getCmdlineMaxBytes, + resolveLinuxProcBudgetMs, + // Exported for white-box tests pinning that the override check and the + // override lookup agree on whitespace-padded GITNEXUS_HOOK_{LSOF,PS}_PATH. + resolveHookBinary, + hasMissingHookBinaryOverride, +}; diff --git a/gitnexus-factory-plugin/hooks/hook-lock.js b/gitnexus-factory-plugin/hooks/hook-lock.js new file mode 100644 index 000000000..05ee20ace --- /dev/null +++ b/gitnexus-factory-plugin/hooks/hook-lock.js @@ -0,0 +1,243 @@ +const crypto = require('crypto'); +const fs = require('fs'); +const path = require('path'); + +const HOOK_LOCK_SUBDIR = '.hook-locks'; +const HOOK_LOCK_MAX_INFLIGHT = 3; +const HOOK_LOCK_STALE_MS = 30000; + +// An evictor's claim marker older than this belongs to a crashed evictor. +// The critical section it guards is a few syscalls (token read, lstat, +// unlink), so any live evictor finishes orders of magnitude sooner; kept well +// under HOOK_LOCK_STALE_MS so an orphan never blocks a slot for long. +const HOOK_LOCK_EVICT_MARKER_STALE_MS = 5000; + +// Same file iff inode identity AND content metadata match. dev+ino alone is +// not enough: filesystems reuse a freed inode number immediately (ext4), so a +// file recreated after an unlink can carry the old file's ino. bigint stats +// keep Windows' 64-bit file ids exact. +function sameSlotFile(a, b) { + return a.dev === b.dev && a.ino === b.ino && a.size === b.size && a.mtimeNs === b.mtimeNs; +} + +function readMarkerToken(marker) { + try { + return fs.readFileSync(marker, 'utf-8'); + } catch { + return null; + } +} + +// Stat and token of a marker, both taken from one open descriptor so they +// describe the same file (a path stat followed by a path read could straddle +// a replacement). O_NOFOLLOW where the platform has it: a marker is always a +// regular file this module created. Returns null when there is no marker. +function readMarkerSnapshot(marker) { + let fd; + try { + fd = fs.openSync(marker, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); + return { stat: fs.fstatSync(fd, { bigint: true }), token: fs.readFileSync(fd, 'utf-8') }; + } catch { + return null; + } finally { + if (fd !== undefined) { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + } + } +} + +// Break an evictor's claim marker only if it is an orphan: older than +// HOOK_LOCK_EVICT_MARKER_STALE_MS, and still the exact file (identity and +// owner token) judged old when it is re-checked just before the unlink. A +// marker released and re-created by a new claimant in between is fresh, so +// it fails the check and stays. +function breakOrphanedMarker(marker) { + const seen = readMarkerSnapshot(marker); + if (!seen || Date.now() - Number(seen.stat.mtimeMs) <= HOOK_LOCK_EVICT_MARKER_STALE_MS) return; + const now = readMarkerSnapshot(marker); + if (!now || !sameSlotFile(now.stat, seen.stat) || now.token !== seen.token) return; + try { + fs.unlinkSync(marker); + } catch { + /* another contender already cleared it */ + } +} + +// Evict a slot judged stale from the `inspected` stat. A slot file is only +// ever deleted, never moved, and only by the evictor holding the per-slot +// `.evicting` marker, created O_EXCL with a token unique to this call. +// Every destructive step verifies first: +// - the slot is unlinked only if the marker still carries our token (an +// evictor stalled long enough for its marker to be broken as an orphan has +// lost its claim and backs off) and the slot is still the exact file +// inspected — identical dev/ino/size/mtimeNs means its content and age are +// unchanged, so the stale verdict still holds, while a slot recreated since +// inspection fails the check and its lock stands; +// - our marker is removed only if it still carries our token, so a marker +// that has passed to another claimant is left alone; +// - an orphaned marker is broken only if it is still the old file it was +// judged to be (see breakOrphanedMarker). +// +// Residual windows. POSIX has no conditional unlink, so each check-then- +// unlink pair keeps a gap of two adjacent syscalls: +// (a) Slot: between the lstat identity check and unlinkSync(slot), a live +// owner past HOOK_LOCK_STALE_MS could release and a new hook recreate the +// slot, whose fresh lock would then be deleted. The consequence is at +// most one extra concurrent augment beyond HOOK_LOCK_MAX_INFLIGHT for +// that run — the cap is a load guard, and no data or index state +// depends on it. The victim's release() sees a foreign or missing file +// and leaves it alone. +// (b) Marker: between the token re-read and unlinkSync(marker) (ours or an +// orphan's), the marker could pass to another claimant, whose claim would +// then be removed. That only re-opens the slot to one more evictor, which +// still has to pass the slot identity check before deleting anything. +// Both need a stall of seconds landing on that exact syscall pair, and the +// only thing lost is one run's cap accounting, so they are accepted rather +// than traded for heavier machinery. A crash at any point orphans at most the +// marker, which the next contender breaks after it expires. +function evictStaleSlot(slotPath, inspected) { + const marker = `${slotPath}.evicting`; + breakOrphanedMarker(marker); + const token = `${process.pid}:${crypto.randomBytes(8).toString('hex')}`; + try { + fs.writeFileSync(marker, token, { flag: 'wx' }); + } catch { + return; // Another evictor holds this slot — leave it to that evictor. + } + try { + if ( + readMarkerToken(marker) === token && + sameSlotFile(fs.lstatSync(slotPath, { bigint: true }), inspected) + ) { + fs.unlinkSync(slotPath); + } + } catch { + /* slot already gone — the retry claims it */ + } finally { + if (readMarkerToken(marker) === token) { + try { + fs.unlinkSync(marker); + } catch { + /* already gone */ + } + } + } +} + +function acquireHookSlot(gitNexusDir) { + const lockDir = path.join(gitNexusDir, HOOK_LOCK_SUBDIR); + try { + fs.mkdirSync(lockDir, { recursive: true }); + } catch { + // Cannot create lock dir (read-only fs, cross-user perm denial, out of + // inodes, etc.) — fail closed by returning null. Caller skips augment. + // Fail-open here would let N concurrent hooks all proceed unguarded and + // reintroduce the #1486 fan-out the guard exists to prevent. + return null; + } + + const myPidStr = String(process.pid); + + for (let slot = 0; slot < HOOK_LOCK_MAX_INFLIGHT; slot++) { + const slotPath = path.join(lockDir, `slot-${slot}.lock`); + for (let attempt = 0; attempt < 2; attempt++) { + try { + fs.writeFileSync(slotPath, myPidStr, { flag: 'wx' }); + let released = false; + const release = () => { + if (released) return; + released = true; + process.removeListener('exit', release); + try { + // Only unlink if we still own the slot. If we appeared stale and + // another hook took over, the file now belongs to it — leave alone. + const content = fs.readFileSync(slotPath, 'utf-8').trim(); + if (content === myPidStr) fs.unlinkSync(slotPath); + } catch { + /* already removed or unreadable */ + } + }; + process.on('exit', release); + return release; + } catch { + // Slot exists. Decide whether to take it over. + // Open once and inspect mtime + content via the same fd so there's + // no TOCTOU between the metadata check and the content read + // (codeql js/file-system-race). + let fd; + try { + fd = fs.openSync(slotPath, 'r'); + } catch { + continue; // Vanished between EEXIST and open — retry this slot. + } + let isLive = false; + let mtimeMs = Date.now(); + let inspected = null; + try { + inspected = fs.fstatSync(fd, { bigint: true }); + mtimeMs = Number(inspected.mtimeMs); + const buf = Buffer.alloc(32); + const n = fs.readSync(fd, buf, 0, 32, 0); + const ownerStr = buf.slice(0, n).toString('utf-8').trim(); + if (ownerStr === '') { + // Owner created the file but hasn't written its PID yet. The + // wx open+write window is microseconds; give it the benefit + // of the doubt and treat as live. + isLive = true; + } else { + const owner = Number.parseInt(ownerStr, 10); + if (Number.isFinite(owner) && owner > 0) { + try { + process.kill(owner, 0); + isLive = true; + } catch (e) { + // ESRCH = process gone → treat as dead. EPERM = process exists + // but owned by another user (cross-user lock dir) → still alive, + // keep the slot. Anything else: be conservative, assume alive. + if (e && e.code === 'ESRCH') { + isLive = false; + } else { + isLive = true; + } + } + } + } + } catch { + /* unreadable — treat as dead */ + } finally { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + } + // For slots younger than HOOK_LOCK_STALE_MS, PID-liveness wins — + // a slow-but-alive hook is never wrongly evicted. For older slots, + // age is the final arbiter as a defense against PID reuse on long- + // abandoned slots. 30s >> the 7s augment timeout, so a healthy run + // never crosses this threshold. + if (isLive && Date.now() - mtimeMs > HOOK_LOCK_STALE_MS) { + isLive = false; + } + if (isLive) break; // Try the next slot. + // No stat means we cannot prove which file we judged stale; leave it + // (the retry re-inspects it) rather than risk deleting a fresh lock. + if (inspected) evictStaleSlot(slotPath, inspected); + // Loop and retry this slot. + } + } + } + + return null; +} + +module.exports = { + HOOK_LOCK_SUBDIR, + HOOK_LOCK_MAX_INFLIGHT, + HOOK_LOCK_STALE_MS, + acquireHookSlot, +}; diff --git a/gitnexus-factory-plugin/hooks/hooks.json b/gitnexus-factory-plugin/hooks/hooks.json new file mode 100644 index 000000000..778a7e747 --- /dev/null +++ b/gitnexus-factory-plugin/hooks/hooks.json @@ -0,0 +1,14 @@ +{ + "PostToolUse": [ + { + "matcher": "Grep|Glob|Execute", + "hooks": [ + { + "type": "command", + "command": "node \"${DROID_PLUGIN_ROOT}/hooks/gitnexus-hook.js\"", + "timeout": 10 + } + ] + } + ] +} diff --git a/gitnexus-factory-plugin/hooks/registry-query.cjs b/gitnexus-factory-plugin/hooks/registry-query.cjs new file mode 100644 index 000000000..5a126d67f --- /dev/null +++ b/gitnexus-factory-plugin/hooks/registry-query.cjs @@ -0,0 +1,405 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { createHash } = require('crypto'); +const { spawnSync } = require('child_process'); + +// Hooks are copied into editor-specific directories and run without the +// package's TypeScript modules. Keep their on-disk names centralized here. +const GITNEXUS_DIR = '.gitnexus'; +const INDEX_METADATA_FILE = 'gitnexus.json'; +const LEGACY_METADATA_FILE = 'meta.json'; +const LBUG_DIRECTORY = 'lbug'; +const BRANCHES_DIRECTORY = 'branches'; +const STORAGE_PATH_ENV = 'GITNEXUS_STORAGE_PATH'; +const STORAGE_ROOT_ENV = 'GITNEXUS_STORAGE_ROOT'; +const STORAGE_SLOT_HASH_LENGTH = 12; +const LOCAL_OWNED_PARENT_HOPS = 5; + +function stripWindowsLongPathPrefix(p) { + if (process.platform !== 'win32') return p; + if (/^\\\\\?\\UNC\\(?=[^\\])/i.test(p)) return `\\\\${p.slice(8)}`; + if (/^\\\\\?\\[A-Za-z]:\\/.test(p)) return p.slice(4); + return p; +} + +function canonicalize(value) { + if (typeof value !== 'string' || !value || value.includes('\0') || !path.isAbsolute(value)) + return null; + const resolved = path.resolve(value); + try { + return stripWindowsLongPathPrefix(fs.realpathSync.native(resolved)); + } catch { + return stripWindowsLongPathPrefix(resolved); + } +} + +function samePath(left, right) { + if (left == null || right == null) return false; + return process.platform === 'win32' ? left.toLowerCase() === right.toLowerCase() : left === right; +} + +function isMissingFile(error) { + return error && (error.code === 'ENOENT' || error.code === 'ENOTDIR'); +} + +function readMetadataFile(storagePath, filename) { + try { + const value = JSON.parse(fs.readFileSync(path.join(storagePath, filename), 'utf-8')); + return value && typeof value === 'object' && !Array.isArray(value) + ? { state: 'valid', value } + : { state: 'invalid' }; + } catch (error) { + return isMissingFile(error) ? { state: 'absent' } : { state: 'invalid' }; + } +} + +function readIndexMetadata(storagePath) { + const primary = readMetadataFile(storagePath, INDEX_METADATA_FILE); + if (primary.state === 'valid') return primary.value; + if (primary.state !== 'absent') return null; + + const legacy = readMetadataFile(storagePath, LEGACY_METADATA_FILE); + return legacy.state === 'valid' ? legacy.value : null; +} + +function isOwnedStorage(repoPath, storagePath, repositoryLocal, metadata) { + // Repository-local storage remains usable for metadata written before + // repoPath was recorded, but an explicit repoPath must never name another + // checkout. External storage always requires the complete ownership binding. + if (repositoryLocal && (!metadata || typeof metadata.repoPath !== 'string')) { + return true; + } + if (!metadata || typeof metadata.repoPath !== 'string') return false; + + const metadataRepoPath = canonicalize(metadata.repoPath); + const expectedRepoPath = canonicalize(repoPath); + if ( + metadataRepoPath == null || + expectedRepoPath == null || + !samePath(metadataRepoPath, expectedRepoPath) + ) { + return false; + } + if (repositoryLocal) return true; + if (typeof metadata.storagePath !== 'string') return false; + + const metadataStoragePath = canonicalize(metadata.storagePath); + const expectedStoragePath = canonicalize(storagePath); + return ( + metadataStoragePath != null && + expectedStoragePath != null && + samePath(metadataStoragePath, expectedStoragePath) + ); +} + +function ancestorPaths(cwd) { + const paths = []; + let current = canonicalize(cwd); + while (current) { + paths.push(current); + const parent = path.dirname(current); + if (parent === current) break; + current = parent; + } + return paths; +} + +function isInsideOrEqual(child, ancestor) { + if (child == null || ancestor == null) return false; + if (samePath(child, ancestor)) return true; + const relative = path.relative(ancestor, child); + return ( + relative !== '' && + relative !== '..' && + !relative.startsWith(`..${path.sep}`) && + !path.isAbsolute(relative) + ); +} + +function ancestorPathsThrough(cwd, stopAt) { + const paths = []; + let current = canonicalize(cwd); + const stop = canonicalize(stopAt); + while (current) { + if (stop && !isInsideOrEqual(current, stop)) break; + paths.push(current); + if (stop && samePath(current, stop)) break; + const parent = path.dirname(current); + if (parent === current) break; + current = parent; + } + return paths; +} + +function currentGitBranch(cwd) { + try { + const result = spawnSync('git', ['symbolic-ref', '--quiet', '--short', 'HEAD'], { + encoding: 'utf-8', + timeout: 2000, + cwd, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + }); + if (result.error || result.status !== 0) return null; + const branch = String(result.stdout || '').trim(); + return branch || null; + } catch { + return null; + } +} + +function registryPathsForCwd(cwd) { + const fallbackPaths = ancestorPaths(cwd); + if (fallbackPaths.length === 0) return { repoPaths: [], branch: null }; + try { + const result = spawnSync( + 'git', + ['rev-parse', '--path-format=absolute', '--show-toplevel', '--git-common-dir'], + { + encoding: 'utf-8', + timeout: 2000, + cwd, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + }, + ); + if (result.error || result.status !== 0) return { repoPaths: fallbackPaths, branch: null }; + + const [worktreeRoot, commonDir] = String(result.stdout || '') + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean); + if (!worktreeRoot || !path.isAbsolute(worktreeRoot)) { + return { repoPaths: fallbackPaths, branch: null }; + } + + // Keep ancestor paths of cwd that stay inside this worktree (cwd up to + // and including show-toplevel) so a --skip-git subdirectory index can + // win via longest-match. Do not walk ancestors outside the worktree — + // that would re-attribute a parent index to a nested git checkout. + const repoPaths = ancestorPathsThrough(cwd, worktreeRoot); + const worktreeCanon = canonicalize(worktreeRoot); + if (worktreeCanon && !repoPaths.some((repoPath) => samePath(repoPath, worktreeCanon))) { + repoPaths.push(worktreeCanon); + } + + // Linked worktrees share the canonical repo's git dir. Include that + // parent so the registered main checkout is still discoverable, but do + // not walk any further outside this worktree. + if (commonDir) { + const commonParent = canonicalize(path.dirname(commonDir)); + if ( + commonParent && + worktreeCanon && + !samePath(commonParent, worktreeCanon) && + !repoPaths.some((repoPath) => samePath(repoPath, commonParent)) + ) { + repoPaths.push(commonParent); + } + } + return { + repoPaths, + branch: currentGitBranch(cwd), + }; + } catch { + return { repoPaths: fallbackPaths, branch: null }; + } +} + +function branchSlug(rawRef) { + const sanitized = rawRef.replace(/^-+/, '').replace(/[^a-zA-Z0-9._-]/g, '_'); + const reserved = /^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])(\..*)?$/i; + const safe = + !sanitized || sanitized === '.' || sanitized === '..' || reserved.test(sanitized) + ? 'unknown' + : sanitized; + const hash = createHash('sha256').update(rawRef).digest('hex').slice(0, 8); + return `${safe}-${hash}`; +} + +// Mirror gitnexus/src/storage/storage-resolver.ts storageSlotName exactly +// (sanitize + sha256 of the canonical repo path, 12-hex suffix). +function sanitizeSlotBasename(value) { + // Cap first, then walk the tail once — same order as + // gitnexus/src/storage/storage-resolver.ts (avoids /[. ]+$/ ReDoS). + const sanitized = value.replace(/[\u0000-\u001f<>:"/\\|?*]/g, '-').slice(0, 80); + let end = sanitized.length; + while (end > 0) { + const code = sanitized.charCodeAt(end - 1); + if (code !== 0x20 && code !== 0x2e) break; + end--; + } + const candidate = sanitized.slice(0, end) || 'repository'; + return /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i.test(candidate) + ? `repository-${candidate}` + : candidate; +} + +function storageSlotName(repoPath) { + const canonical = canonicalize(repoPath); + if (!canonical) return null; + const identity = process.platform === 'win32' ? canonical.toLowerCase() : canonical; + const basename = sanitizeSlotBasename(path.basename(canonical)); + const digest = createHash('sha256') + .update(identity) + .digest('hex') + .slice(0, STORAGE_SLOT_HASH_LENGTH); + return `${basename}-${digest}`; +} + +// Definedness matches the CLI's storage-resolver.ts: any value other than +// undefined (including '') counts as configured. +function envOverridesStorage() { + return process.env[STORAGE_PATH_ENV] !== undefined || process.env[STORAGE_ROOT_ENV] !== undefined; +} + +// A set-but-invalid override (empty, relative, or containing NUL) makes +// storage unresolvable (the CLI's storage-resolver.ts throws); never fall +// back to the registry row. A filesystem root is invalid only for +// GITNEXUS_STORAGE_PATH (validateConfiguredStoragePath rejects it); +// GITNEXUS_STORAGE_ROOT accepts a filesystem root — storagePathFromRoot +// resolves the slot directly under it. +function resolveEntryStoragePath(entry) { + const envPath = process.env[STORAGE_PATH_ENV]; + if (envPath !== undefined) { + if (!envPath || envPath.includes('\0') || !path.isAbsolute(envPath)) return null; + const resolved = path.resolve(envPath); + // validateConfiguredStoragePath rejects a filesystem root. + return path.basename(resolved) ? resolved : null; + } + + const envRoot = process.env[STORAGE_ROOT_ENV]; + if (envRoot !== undefined) { + if (!envRoot || envRoot.includes('\0') || !path.isAbsolute(envRoot)) return null; + const root = path.resolve(envRoot); + const slot = storageSlotName(entry.path); + if (!slot) return null; + const storagePath = path.join(root, slot); + return samePath(path.dirname(storagePath), root) ? storagePath : null; + } + + if (entry.storagePath !== undefined) { + if ( + typeof entry.storagePath !== 'string' || + !entry.storagePath || + entry.storagePath.includes('\0') || + !path.isAbsolute(entry.storagePath) + ) { + return null; + } + return path.resolve(entry.storagePath); + } + return path.resolve(path.join(entry.path, GITNEXUS_DIR)); +} + +function hasLocalIndexSignal(storagePath) { + try { + return ( + fs.existsSync(path.join(storagePath, INDEX_METADATA_FILE)) || + fs.existsSync(path.join(storagePath, LBUG_DIRECTORY)) + ); + } catch { + return false; + } +} + +function findLocalOwnedRepo(cwd) { + // Environment storage overrides win; a leftover repo-local .gitnexus must + // not skip the registry scan that applies STORAGE_PATH / STORAGE_ROOT. + if (envOverridesStorage()) return null; + const { repoPaths, branch } = registryPathsForCwd(cwd); + let current = canonicalize(cwd); + for (let hops = 0; hops <= LOCAL_OWNED_PARENT_HOPS && current; hops++) { + const storagePath = path.join(current, GITNEXUS_DIR); + if (hasLocalIndexSignal(storagePath)) { + const metadata = readIndexMetadata(storagePath); + if (isOwnedStorage(current, storagePath, true, metadata)) { + const branchDir = + branch != null ? path.join(storagePath, BRANCHES_DIRECTORY, branchSlug(branch)) : null; + const indexDir = branchDir && hasLocalIndexSignal(branchDir) ? branchDir : storagePath; + return { + path: current, + storagePath, + lbugPath: path.join(indexDir, LBUG_DIRECTORY), + metadata: indexDir === storagePath ? metadata : readIndexMetadata(indexDir), + }; + } + } + const parent = path.dirname(current); + if (parent === current) break; + // Stay inside this checkout. Registered lookup already stops at + // `--show-toplevel`; walking raw parents would adopt `/outer/.gitnexus` + // from `/outer/nested-repo`. + if (repoPaths.length > 0 && !repoPaths.some((repoPath) => samePath(repoPath, parent))) { + break; + } + current = parent; + } + return null; +} + +function findRegisteredRepo(cwd) { + const { repoPaths, branch } = registryPathsForCwd(cwd); + if (repoPaths.length === 0) return null; + + const home = process.env.GITNEXUS_HOME || path.join(os.homedir(), '.gitnexus'); + let entries; + try { + entries = JSON.parse(fs.readFileSync(path.join(home, 'registry.json'), 'utf-8')); + } catch { + return null; + } + if (!Array.isArray(entries)) return null; + + let best = null; + let bestLen = -1; + for (const entry of entries) { + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) continue; + if (typeof entry.path !== 'string') continue; + if (entry.path.includes('\0') || !path.isAbsolute(entry.path)) continue; + const registeredPath = canonicalize(entry.path); + if (!registeredPath || !repoPaths.some((repoPath) => samePath(repoPath, registeredPath))) { + continue; + } + const storagePath = resolveEntryStoragePath(entry); + if (!storagePath) continue; + const repositoryLocal = samePath( + canonicalize(path.join(entry.path, GITNEXUS_DIR)), + canonicalize(storagePath), + ); + const ownershipMetadata = readIndexMetadata(storagePath); + if (!isOwnedStorage(entry.path, storagePath, repositoryLocal, ownershipMetadata)) continue; + const branchIsIndexed = + branch && + Array.isArray(entry.branches) && + entry.branches.some((summary) => summary && summary.branch === branch); + const indexDir = branchIsIndexed + ? path.join(storagePath, BRANCHES_DIRECTORY, branchSlug(branch)) + : storagePath; + if (registeredPath.length > bestLen) { + bestLen = registeredPath.length; + best = { + path: entry.path, + storagePath, + lbugPath: path.join(indexDir, LBUG_DIRECTORY), + metadata: branchIsIndexed ? readIndexMetadata(indexDir) : ownershipMetadata, + }; + } + } + return best; +} + +/** Registry row wins (including persisted external storagePath); local owned is fallback. */ +function resolveHookRepo(cwd) { + return findRegisteredRepo(cwd) || findLocalOwnedRepo(cwd); +} + +module.exports = { + findRegisteredRepo, + findLocalOwnedRepo, + resolveHookRepo, + INDEX_METADATA_FILE, + LEGACY_METADATA_FILE, + LBUG_DIRECTORY, +}; diff --git a/gitnexus-factory-plugin/hooks/win-rm-list-json.ps1 b/gitnexus-factory-plugin/hooks/win-rm-list-json.ps1 new file mode 100644 index 000000000..5c1564e30 --- /dev/null +++ b/gitnexus-factory-plugin/hooks/win-rm-list-json.ps1 @@ -0,0 +1,76 @@ +$ErrorActionPreference = 'Stop' +$target = $env:GITNEXUS_HOOK_RM_TARGET +if ([string]::IsNullOrWhiteSpace($target)) { Write-Output '[]'; exit 0 } +$target = (Resolve-Path -LiteralPath $target).ProviderPath + +if (-not ([Management.Automation.PSTypeName]'GitNexusHookRm.Native').Type) { +Add-Type @' +using System; +using System.Runtime.InteropServices; +namespace GitNexusHookRm { + public static class Native { + public const int ErrorMoreData = 234; + [StructLayout(LayoutKind.Sequential, Pack = 4)] + public struct RM_UNIQUE_PROCESS { + public int dwProcessId; + public long ProcessStartTime; + } + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] + public struct RM_PROCESS_INFO { + public RM_UNIQUE_PROCESS Process; + [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 256)] + public string strAppName; + [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 64)] + public string strServiceShortName; + public uint ApplicationType; + public uint AppStatus; + public uint TSSessionId; + public uint bRestartable; + } + [DllImport("rstrtmgr.dll", CharSet = CharSet.Unicode)] + public static extern int RmStartSession(out uint pSessionHandle, uint dwSessionFlags, string strSessionKey); + [DllImport("rstrtmgr.dll", CharSet = CharSet.Unicode)] + public static extern int RmRegisterResources(uint pSessionHandle, uint nFiles, string[] rgsFileNames, uint nApplications, IntPtr rgApplications, uint nServices, string[] rgsServiceNames); + [DllImport("rstrtmgr.dll")] + public static extern int RmGetList(uint dwSessionHandle, out uint pnProcInfoNeeded, ref uint pnProcInfo, [In, Out] RM_PROCESS_INFO[] rgAffectedApps, ref uint lpdwRebootReasons); + [DllImport("rstrtmgr.dll")] + public static extern int RmEndSession(uint pSessionHandle); + } +} +'@ +} + +$h = [uint32]0 +$key = [guid]::NewGuid().ToString('N') +$rmErr = [GitNexusHookRm.Native]::RmStartSession([ref]$h, 0, $key) +if ($rmErr -ne 0) { Write-Output '[]'; exit 0 } +$files = @($target) +$err = [GitNexusHookRm.Native]::RmRegisterResources($h, 1, $files, 0, [IntPtr]::Zero, 0, $null) +if ($err -ne 0) { + [void][GitNexusHookRm.Native]::RmEndSession($h) + Write-Output '[]' + exit 0 +} +$need = [uint32]0 +$n = [uint32]0 +$reboot = [uint32]0 +$err = [GitNexusHookRm.Native]::RmGetList($h, [ref]$need, [ref]$n, $null, [ref]$reboot) +if ($err -ne [GitNexusHookRm.Native]::ErrorMoreData) { + [void][GitNexusHookRm.Native]::RmEndSession($h) + Write-Output '[]' + exit 0 +} +$n = $need +$buf = New-Object GitNexusHookRm.Native+RM_PROCESS_INFO[] ([int]$n) +$err = [GitNexusHookRm.Native]::RmGetList($h, [ref]$need, [ref]$n, $buf, [ref]$reboot) +[void][GitNexusHookRm.Native]::RmEndSession($h) +if ($err -ne 0) { Write-Output '[]'; exit 0 } + +$out = @() +for ($i = 0; $i -lt [int]$n; $i++) { + $procId = $buf[$i].Process.dwProcessId + $p = Get-CimInstance -ClassName Win32_Process -Filter "ProcessId=$procId" -ErrorAction SilentlyContinue + $cmd = if ($p) { $p.CommandLine } else { '' } + $out += [PSCustomObject]@{ pid = [int]$procId; cmd = $cmd } +} +ConvertTo-Json -InputObject @($out) -Compress diff --git a/gitnexus-factory-plugin/mcp.json b/gitnexus-factory-plugin/mcp.json new file mode 100644 index 000000000..afe364b82 --- /dev/null +++ b/gitnexus-factory-plugin/mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "gitnexus": { + "command": "npx", + "args": ["-y", "gitnexus@1.6.12", "mcp"] + } + } +} diff --git a/gitnexus/README.md b/gitnexus/README.md index 6a0866733..95e84db61 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -2,7 +2,7 @@ **Graph-powered code intelligence for AI agents.** Index any codebase into a knowledge graph, then query it via MCP or CLI. -Works with **Cursor**, **Claude Code**, **Antigravity** (Google), **Codex**, **Windsurf**, **Cline**, **OpenCode**, **CodeBuddy** (Tencent), **Qoder** (Alibaba), and any MCP-compatible tool. +Works with **Cursor**, **Claude Code**, **Antigravity** (Google), **Codex**, **Factory** (Droid), **Windsurf**, **Cline**, **OpenCode**, **CodeBuddy** (Tencent), **Qoder** (Alibaba), and any MCP-compatible tool. [![npm version](https://img.shields.io/npm/v/gitnexus.svg)](https://www.npmjs.com/package/gitnexus) [![License: PolyForm Noncommercial](https://img.shields.io/badge/License-PolyForm%20Noncommercial-blue.svg)](https://polyformproject.org/licenses/noncommercial/1.0.0/) @@ -44,12 +44,13 @@ To configure MCP for your editor, run `npx gitnexus setup` once — or set it up | **Cursor** | Yes | Yes | Yes (postToolUse, [manual install](../gitnexus-cursor-integration/README.md#hook-install)) | **Full** | | **Antigravity** (Google) | Yes | Yes | Yes (AfterTool, [Gemini CLI hooks schema](https://geminicli.com/docs/hooks/reference/)) | **Full** | | **Codex** | Yes | Yes | Yes (PreToolUse + PostToolUse, [Codex hooks](https://developers.openai.com/codex/hooks)) | **Full** | +| **Factory** (Droid) | Yes | Yes | Yes (PostToolUse, [plugin](../gitnexus-factory-plugin/)) | **Full** | | **OpenCode** | Yes | Yes | — | MCP + Skills | | **CodeBuddy** (Tencent) | Yes | Yes | — | MCP + Skills | | **Qoder** (Alibaba) | Yes | Yes | — | MCP + Skills | | **Windsurf** | Yes | — | — | MCP | -> **Claude Code** and **Codex** get the deepest integration: MCP tools + agent skills + PreToolUse hooks that automatically enrich grep/glob/bash calls with knowledge graph context + PostToolUse hooks that detect a stale index after commits and prompt the agent to reindex. +> **Full** means MCP tools + agent skills + hooks that enrich searches with graph context. **Claude Code** and **Codex** go deepest: their PreToolUse hooks enrich the search before it runs, and their PostToolUse hooks also detect a stale index after commits and prompt the agent to reindex. **Cursor**, **Antigravity**, and **Factory** augment from a post-tool hook only, so they enrich the result rather than the query and do not carry the stale-index hint. ### Community Integrations @@ -88,6 +89,33 @@ codex plugin marketplace add abhigyanpatwari/GitNexus > **Codex notes:** SessionStart is intentionally not registered — Codex reads [AGENTS.md natively](https://developers.openai.com/codex/guides/agents-md), which already carries the GitNexus context block. Newly installed hooks need a one-time approval in Codex via `/hooks` before they run. Pick **one** install route (`gitnexus setup -c codex` **or** the plugin): plugin hooks load alongside `~/.codex/hooks.json`, so installing both can fire duplicate hooks per tool call. +### Factory (Droid) (full support — MCP + skills + hooks) + +`gitnexus setup -c droid` writes the MCP server to `~/.factory/mcp.json` and installs skills to +`~/.factory/skills/`. To configure MCP by hand instead, add to `~/.factory/mcp.json` +([user scope](https://docs.factory.ai/cli/configuration/mcp) — applies to all projects): + +```json +{ + "mcpServers": { + "gitnexus": { + "command": "npx", + "args": ["-y", "gitnexus@latest", "mcp"] + } + } +} +``` + +For the PostToolUse search-augment hook, install the bundled +[`gitnexus-factory-plugin/`](../gitnexus-factory-plugin/) with `droid plugin install gitnexus@` +from a marketplace that includes this repo, or point Droid at it via `extraKnownMarketplaces` in +`.factory/settings.json`. + +> **Factory notes:** Droid reads [AGENTS.md natively](https://docs.factory.ai/), which already carries the +> GitNexus context block, so no SessionStart hook is registered. Pick **one** install route +> (`gitnexus setup -c droid` **or** the plugin) — the plugin ships its own MCP entry, so installing both +> can register the server twice. + ### Cursor / Windsurf Add to `~/.cursor/mcp.json` (global — works for all projects): diff --git a/gitnexus/hooks/claude/hook-db-lock-probe.cjs b/gitnexus/hooks/claude/hook-db-lock-probe.cjs index 4376795e2..3e98f5901 100644 --- a/gitnexus/hooks/claude/hook-db-lock-probe.cjs +++ b/gitnexus/hooks/claude/hook-db-lock-probe.cjs @@ -80,9 +80,11 @@ function debugLog(msg) { function resolveHookBinary(tool) { const envKey = tool === 'lsof' ? 'GITNEXUS_HOOK_LSOF_PATH' : 'GITNEXUS_HOOK_PS_PATH'; - const fromEnv = process.env[envKey]; - if (fromEnv && String(fromEnv).trim() && fs.existsSync(String(fromEnv))) { - return String(fromEnv); + // Trim once, exactly as hasMissingHookBinaryOverride does, so a padded but + // valid override (" /tmp/lsof ") is both accepted there and used here. + const fromEnv = process.env[envKey] ? String(process.env[envKey]).trim() : ''; + if (fromEnv && fs.existsSync(fromEnv)) { + return fromEnv; } const candidates = tool === 'lsof' @@ -177,7 +179,13 @@ function resolveUnixGuardTimeout() { const trimmed = fromEnv ? String(fromEnv).trim() : ''; if (trimmed === 'disabled') return unixGuardTimeoutCache; const candidates = []; - if (trimmed && fs.existsSync(trimmed)) candidates.push(trimmed); + // Resolve the override against THIS process's cwd — the directory the + // existsSync check and the self-test run in — so the cached/returned path is + // always absolute. The adapters spawn the wrapper with a different `cwd` + // (the tool request's), where a relative value would resolve elsewhere + // (ENOENT), and a slashless name would switch to a PATH lookup. + const override = trimmed ? path.resolve(trimmed) : ''; + if (override && fs.existsSync(override)) candidates.push(override); for (const builtin of [ '/usr/bin/timeout', '/bin/timeout', @@ -317,15 +325,30 @@ function getProcRoot() { // `node mcp` line // (the `mcp`/`serve` mode token lives at the very tail, so the cap must be large // enough to reach it — see PROC_CMDLINE_FLOOR escalation below). Overridable for -// tests; never goes below PROC_CMDLINE_FLOOR. +// tests via GITNEXUS_HOOK_PROC_CMDLINE_MAX: an integer in +// [PROC_CMDLINE_FLOOR, PROC_CMDLINE_CEIL] is used as-is; a larger integer is +// CLAMPED to PROC_CMDLINE_CEIL; anything else (below the floor, fractional, +// non-numeric, Infinity) falls back to the 16 KiB default. const PROC_CMDLINE_FLOOR = 4096; +// Upper bound for a single cmdline read chunk, and the absolute ceiling of the +// escalation path in readLinuxCmdline (same 256 KiB — no single read may exceed +// what the whole escalation is allowed to collect). Without it, an oversized +// override (e.g. 2**40 — past buffer.constants.MAX_LENGTH on older Node lines +// and unallocatable in practice on any) made Buffer.allocUnsafe throw; readLinuxCmdline's catch turned that into '' (a +// NON-candidate), so a real server owner was silently missed (fail-OPEN, the +// #1492 race). Oversized values are clamped rather than defaulted: the operator +// asked for MORE bytes, and the ceiling is the most the read will ever collect +// anyway, so clamping honours the intent while keeping allocation bounded. +const PROC_CMDLINE_CEIL = 262144; function getCmdlineMaxBytes() { const raw = process.env.GITNEXUS_HOOK_PROC_CMDLINE_MAX; // Number() (not parseInt) so "8e3" reads as 8000, not 8 (parseInt stops at // 'e'). The `raw && String(raw).trim()` guard keeps empty/whitespace on the // default; trailing garbage ("8abc") now -> NaN -> default (stricter). const n = raw && String(raw).trim() ? Number(String(raw).trim()) : NaN; - if (Number.isFinite(n) && n >= PROC_CMDLINE_FLOOR) return n; + // Number.isInteger rejects NaN, +/-Infinity and fractions (a fractional + // Buffer/readSync length is not a byte count). + if (Number.isInteger(n) && n >= PROC_CMDLINE_FLOOR) return Math.min(n, PROC_CMDLINE_CEIL); return 16384; } @@ -381,19 +404,24 @@ const CMDLINE_TIMEOUT = Symbol('gitnexus.cmdline.timeout'); // Bounded /proc//cmdline read for Phase 1. openSync+readSync (not // readFileSync) so a D-state holder cannot stall the hook on a huge or -// never-EOF argv: we read at most `cap` bytes and stop. cmdline separates argv -// with NULs; convert to spaces for isGitNexusServerCommand. +// never-EOF argv: we read in `cap`-sized chunks and stop as soon as the text +// holds both server tokens, at EOF, at PROC_CMDLINE_CEIL, or when the scan +// budget runs out (see below). cmdline separates argv with NULs; convert to +// spaces for isGitNexusServerCommand. // // Owner-miss guard for the 4 KB cap: the `gitnexus` token usually sits in the // first path component while the `mcp`/`serve` mode token is the LAST argv, so // a naive 4 KB read could clip the mode token off a server launched with a very // long interpreter path and silently miss a real owner. We mitigate two ways: -// (a) the default cap (16 KiB) already clears realistic lines; (b) if the first -// read fills the cap AND already contains the `gitnexus` token but no mode -// token yet, we keep reading in bounded chunks (up to a hard ceiling) until the -// mode token appears or the file ends — so a genuine server is never missed for -// want of a few more bytes, while non-candidates still pay only the initial -// bounded read. +// (a) the default cap (16 KiB) already clears realistic lines, so almost every +// process is decided by the first read hitting EOF; (b) a read that fills the +// cap stops early ONLY once it holds BOTH tokens (decided owner). Holding one +// token, or neither, decides nothing: interpreter flags can put a mode-looking +// word first (`node --require mcp .../gitnexus/... serve`) or push the gitnexus +// path past the first chunk. So we keep reading in cap-sized chunks until both +// tokens appear, the file ends, or the hard ceiling is reached. Only processes +// that passed the Phase 0 comm prefilter AND have a cmdline longer than the cap +// ever escalate, and each escalation step is budget-gated (below). // // Budget (F3): the escalation loop above is the one place a SINGLE pathological // candidate could read up to HARD_CEIL (256 KiB) before the next scan-level @@ -411,7 +439,7 @@ function readLinuxCmdline(procRoot, pidStr, cap, outOfBudget) { return ''; } try { - const HARD_CEIL = 262144; // 256 KiB absolute ceiling for the escalation path + const HARD_CEIL = PROC_CMDLINE_CEIL; // 256 KiB absolute ceiling for the escalation path let collected = Buffer.alloc(0); let offset = 0; let chunkCap = cap; @@ -425,16 +453,12 @@ function readLinuxCmdline(procRoot, pidStr, cap, outOfBudget) { collected = Buffer.concat([collected, buf.subarray(0, bytes)]); offset += bytes; const text = collected.toString('utf8').replace(/\0+/g, ' '); - // Stop early when we can already decide "owner": has both the gitnexus - // token and a mode token. Keep going only when gitnexus is present but - // the mode token might be just past the boundary. - const hasGitNexus = - /(?:^|[/\\\s])gitnexus(?:\.cmd)?(?:\s|$)/.test(text) || - /node_modules[/\\]gitnexus[/\\]/.test(text); - const hasMode = /(?:^|\s)(mcp|serve)(?:\s|$)/.test(text); - if (hasMode) break; // decided (positive); isGitNexusServerCommand re-checks below + // Stop early only when the partial read is DECIDED: both the gitnexus + // token and a mode token are present (isGitNexusServerCommand is exactly + // that conjunction). A partial read missing either token is undecided — + // the missing one may lie past the chunk boundary — so it keeps reading. + if (isGitNexusServerCommand(text)) break; // decided (positive) if (bytes < chunkCap) break; // EOF: full cmdline read, definitive - if (!hasGitNexus) break; // not a candidate; do not escalate the read if (offset >= HARD_CEIL) break; // bounded escalation only // Budget gate the escalation: a single huge-argv candidate must not burn // the whole scan deadline before we re-check. Return the timeout sentinel @@ -578,17 +602,24 @@ function linuxProcScanFindGitNexusServer(dbPathAbs, myPid) { ); return 'timeout'; } - // Any other shape (ENOTDIR — fd path is not a directory at all, so this - // is not a plausible live-procfs owner — and the long tail) is treated as - // "this candidate is not an owner": move to the next candidate instead of - // the old blanket 'owned'. If no other candidate owns the lbug the scan - // ends not-owned (dispatcher fail-open) — acceptable because ENOTDIR means - // the fd entry is structurally not a real /proc//fd. + if (code === 'ENOTDIR') { + // The fd path is not a directory at all, so this is structurally not a + // real /proc//fd — not a plausible live owner. Move on. + debugLog( + `fd dir not a directory for candidate pid ${pidStr} (ENOTDIR); ` + + `treating candidate as non-owner -> continue`, + ); + continue; + } + // Any other error (EMFILE, ENFILE, ENOMEM, EINTR, no code, …) says nothing + // about whether this already-identified server candidate holds the lbug. + // Only ENOENT/ENOTDIR above establish non-ownership; everything else is + // inconclusive and fails closed via 'timeout', same as EACCES/EIO. debugLog( - `fd dir not a readable directory for candidate pid ${pidStr} ` + - `(${code || 'unknown'}); treating candidate as non-owner -> continue`, + `fd dir read failed for candidate pid ${pidStr} ` + + `(${code || 'unknown'}); probe inconclusive -> fail-closed (timeout)`, ); - continue; + return 'timeout'; } for (const fd of fds) { if (outOfBudget()) return 'timeout'; @@ -707,16 +738,12 @@ module.exports = { // name is pinned by a source-contract test. linuxProcScanFindGitNexusServer, // #2163 follow-up: the hook adapters wrap the augment CLI in the same - // guard. Returns a self-tested wrapper path — the built-in candidates are - // always absolute; a GITNEXUS_HOOK_TIMEOUT_PATH override is adopted as the - // exact string that passed the self-test. Same string is also the same - // RESOLUTION for absolute paths and for slashless names (PATH lookup is - // cwd-independent); a slash-containing RELATIVE override, however, is - // existsSync-checked and self-tested against this process's cwd while the - // adapters spawn the CLI with a `cwd` option (chdir-before-exec), so such - // a value can pass here yet ENOENT at the augment call site — set the - // override to an absolute path. Returns null when the wrapper is - // disabled/unavailable. Never call on win32 (see its JSDoc). + // guard. Returns a self-tested, always-ABSOLUTE wrapper path: the built-in + // candidates are absolute, and a GITNEXUS_HOOK_TIMEOUT_PATH override is + // path.resolve()d against this process's cwd before its existsSync check + // and self-test, so the adapters can spawn it under any `cwd` option. + // Returns null when the wrapper is disabled/unavailable. Never call on + // win32 (see its JSDoc). resolveUnixGuardTimeout, // Exported for white-box unit tests of the numeric-env parsing (#2183 review): // Number()-not-parseInt so "16e3" reads as 16000, plus the empty/whitespace @@ -725,4 +752,8 @@ module.exports = { // otherwise only observable indirectly through scan timing/escalation. getCmdlineMaxBytes, resolveLinuxProcBudgetMs, + // Exported for white-box tests pinning that the override check and the + // override lookup agree on whitespace-padded GITNEXUS_HOOK_{LSOF,PS}_PATH. + resolveHookBinary, + hasMissingHookBinaryOverride, }; diff --git a/gitnexus/hooks/claude/hook-lock.cjs b/gitnexus/hooks/claude/hook-lock.cjs index 759856384..05ee20ace 100644 --- a/gitnexus/hooks/claude/hook-lock.cjs +++ b/gitnexus/hooks/claude/hook-lock.cjs @@ -1,3 +1,4 @@ +const crypto = require('crypto'); const fs = require('fs'); const path = require('path'); @@ -5,6 +6,128 @@ const HOOK_LOCK_SUBDIR = '.hook-locks'; const HOOK_LOCK_MAX_INFLIGHT = 3; const HOOK_LOCK_STALE_MS = 30000; +// An evictor's claim marker older than this belongs to a crashed evictor. +// The critical section it guards is a few syscalls (token read, lstat, +// unlink), so any live evictor finishes orders of magnitude sooner; kept well +// under HOOK_LOCK_STALE_MS so an orphan never blocks a slot for long. +const HOOK_LOCK_EVICT_MARKER_STALE_MS = 5000; + +// Same file iff inode identity AND content metadata match. dev+ino alone is +// not enough: filesystems reuse a freed inode number immediately (ext4), so a +// file recreated after an unlink can carry the old file's ino. bigint stats +// keep Windows' 64-bit file ids exact. +function sameSlotFile(a, b) { + return a.dev === b.dev && a.ino === b.ino && a.size === b.size && a.mtimeNs === b.mtimeNs; +} + +function readMarkerToken(marker) { + try { + return fs.readFileSync(marker, 'utf-8'); + } catch { + return null; + } +} + +// Stat and token of a marker, both taken from one open descriptor so they +// describe the same file (a path stat followed by a path read could straddle +// a replacement). O_NOFOLLOW where the platform has it: a marker is always a +// regular file this module created. Returns null when there is no marker. +function readMarkerSnapshot(marker) { + let fd; + try { + fd = fs.openSync(marker, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); + return { stat: fs.fstatSync(fd, { bigint: true }), token: fs.readFileSync(fd, 'utf-8') }; + } catch { + return null; + } finally { + if (fd !== undefined) { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + } + } +} + +// Break an evictor's claim marker only if it is an orphan: older than +// HOOK_LOCK_EVICT_MARKER_STALE_MS, and still the exact file (identity and +// owner token) judged old when it is re-checked just before the unlink. A +// marker released and re-created by a new claimant in between is fresh, so +// it fails the check and stays. +function breakOrphanedMarker(marker) { + const seen = readMarkerSnapshot(marker); + if (!seen || Date.now() - Number(seen.stat.mtimeMs) <= HOOK_LOCK_EVICT_MARKER_STALE_MS) return; + const now = readMarkerSnapshot(marker); + if (!now || !sameSlotFile(now.stat, seen.stat) || now.token !== seen.token) return; + try { + fs.unlinkSync(marker); + } catch { + /* another contender already cleared it */ + } +} + +// Evict a slot judged stale from the `inspected` stat. A slot file is only +// ever deleted, never moved, and only by the evictor holding the per-slot +// `.evicting` marker, created O_EXCL with a token unique to this call. +// Every destructive step verifies first: +// - the slot is unlinked only if the marker still carries our token (an +// evictor stalled long enough for its marker to be broken as an orphan has +// lost its claim and backs off) and the slot is still the exact file +// inspected — identical dev/ino/size/mtimeNs means its content and age are +// unchanged, so the stale verdict still holds, while a slot recreated since +// inspection fails the check and its lock stands; +// - our marker is removed only if it still carries our token, so a marker +// that has passed to another claimant is left alone; +// - an orphaned marker is broken only if it is still the old file it was +// judged to be (see breakOrphanedMarker). +// +// Residual windows. POSIX has no conditional unlink, so each check-then- +// unlink pair keeps a gap of two adjacent syscalls: +// (a) Slot: between the lstat identity check and unlinkSync(slot), a live +// owner past HOOK_LOCK_STALE_MS could release and a new hook recreate the +// slot, whose fresh lock would then be deleted. The consequence is at +// most one extra concurrent augment beyond HOOK_LOCK_MAX_INFLIGHT for +// that run — the cap is a load guard, and no data or index state +// depends on it. The victim's release() sees a foreign or missing file +// and leaves it alone. +// (b) Marker: between the token re-read and unlinkSync(marker) (ours or an +// orphan's), the marker could pass to another claimant, whose claim would +// then be removed. That only re-opens the slot to one more evictor, which +// still has to pass the slot identity check before deleting anything. +// Both need a stall of seconds landing on that exact syscall pair, and the +// only thing lost is one run's cap accounting, so they are accepted rather +// than traded for heavier machinery. A crash at any point orphans at most the +// marker, which the next contender breaks after it expires. +function evictStaleSlot(slotPath, inspected) { + const marker = `${slotPath}.evicting`; + breakOrphanedMarker(marker); + const token = `${process.pid}:${crypto.randomBytes(8).toString('hex')}`; + try { + fs.writeFileSync(marker, token, { flag: 'wx' }); + } catch { + return; // Another evictor holds this slot — leave it to that evictor. + } + try { + if ( + readMarkerToken(marker) === token && + sameSlotFile(fs.lstatSync(slotPath, { bigint: true }), inspected) + ) { + fs.unlinkSync(slotPath); + } + } catch { + /* slot already gone — the retry claims it */ + } finally { + if (readMarkerToken(marker) === token) { + try { + fs.unlinkSync(marker); + } catch { + /* already gone */ + } + } + } +} + function acquireHookSlot(gitNexusDir) { const lockDir = path.join(gitNexusDir, HOOK_LOCK_SUBDIR); try { @@ -28,6 +151,7 @@ function acquireHookSlot(gitNexusDir) { const release = () => { if (released) return; released = true; + process.removeListener('exit', release); try { // Only unlink if we still own the slot. If we appeared stale and // another hook took over, the file now belongs to it — leave alone. @@ -52,8 +176,10 @@ function acquireHookSlot(gitNexusDir) { } let isLive = false; let mtimeMs = Date.now(); + let inspected = null; try { - mtimeMs = fs.fstatSync(fd).mtimeMs; + inspected = fs.fstatSync(fd, { bigint: true }); + mtimeMs = Number(inspected.mtimeMs); const buf = Buffer.alloc(32); const n = fs.readSync(fd, buf, 0, 32, 0); const ownerStr = buf.slice(0, n).toString('utf-8').trim(); @@ -98,11 +224,9 @@ function acquireHookSlot(gitNexusDir) { isLive = false; } if (isLive) break; // Try the next slot. - try { - fs.unlinkSync(slotPath); - } catch { - /* another hook beat us to it — retry will hit EEXIST */ - } + // No stat means we cannot prove which file we judged stale; leave it + // (the retry re-inspects it) rather than risk deleting a fresh lock. + if (inspected) evictStaleSlot(slotPath, inspected); // Loop and retry this slot. } } diff --git a/gitnexus/hooks/claude/registry-query.cjs b/gitnexus/hooks/claude/registry-query.cjs index f5b2a0576..0607602db 100644 --- a/gitnexus/hooks/claude/registry-query.cjs +++ b/gitnexus/hooks/claude/registry-query.cjs @@ -248,40 +248,35 @@ function storageSlotName(repoPath) { return `${basename}-${digest}`; } +// Definedness matches the CLI's storage-resolver.ts: any value other than +// undefined (including '') counts as configured. function envOverridesStorage() { - const envPath = process.env[STORAGE_PATH_ENV]; - const envRoot = process.env[STORAGE_ROOT_ENV]; - return ( - (typeof envPath === 'string' && envPath.length > 0) || - (typeof envRoot === 'string' && envRoot.length > 0) - ); + return process.env[STORAGE_PATH_ENV] !== undefined || process.env[STORAGE_ROOT_ENV] !== undefined; } +// A set-but-invalid override (empty, relative, or containing NUL) makes +// storage unresolvable (the CLI's storage-resolver.ts throws); never fall +// back to the registry row. A filesystem root is invalid only for +// GITNEXUS_STORAGE_PATH (validateConfiguredStoragePath rejects it); +// GITNEXUS_STORAGE_ROOT accepts a filesystem root — storagePathFromRoot +// resolves the slot directly under it. function resolveEntryStoragePath(entry) { const envPath = process.env[STORAGE_PATH_ENV]; - if ( - typeof envPath === 'string' && - envPath.length > 0 && - !envPath.includes('\0') && - path.isAbsolute(envPath) - ) { + if (envPath !== undefined) { + if (!envPath || envPath.includes('\0') || !path.isAbsolute(envPath)) return null; const resolved = path.resolve(envPath); - if (path.isAbsolute(resolved)) return resolved; + // validateConfiguredStoragePath rejects a filesystem root. + return path.basename(resolved) ? resolved : null; } const envRoot = process.env[STORAGE_ROOT_ENV]; - if ( - typeof envRoot === 'string' && - envRoot.length > 0 && - !envRoot.includes('\0') && - path.isAbsolute(envRoot) - ) { + if (envRoot !== undefined) { + if (!envRoot || envRoot.includes('\0') || !path.isAbsolute(envRoot)) return null; const root = path.resolve(envRoot); const slot = storageSlotName(entry.path); - if (slot) { - const storagePath = path.join(root, slot); - if (samePath(path.dirname(storagePath), root)) return storagePath; - } + if (!slot) return null; + const storagePath = path.join(root, slot); + return samePath(path.dirname(storagePath), root) ? storagePath : null; } if (entry.storagePath !== undefined) { diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index de688cf5a..be3adf782 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -108,6 +108,7 @@ const PLATFORM_LOGIC = [ 'test/unit/hooks.test.ts', 'test/unit/hook-db-lock-probe.test.ts', 'test/unit/cursor-hook.test.ts', + 'test/unit/factory-plugin.test.ts', 'test/unit/sidecar-recovery.test.ts', 'test/unit/pool-wal-recovery.test.ts', 'test/unit/lbug-adapter-wal-schema.test.ts', diff --git a/gitnexus/scripts/sync-plugin-manifests.mjs b/gitnexus/scripts/sync-plugin-manifests.mjs index 5bcc8f29c..3858cfd06 100644 --- a/gitnexus/scripts/sync-plugin-manifests.mjs +++ b/gitnexus/scripts/sync-plugin-manifests.mjs @@ -13,6 +13,9 @@ * - gitnexus-claude-plugin/.codex-plugin/plugin.json (top-level version) * - .agents/plugins/marketplace.json (plugins[gitnexus]) * - gitnexus-claude-plugin/skills//mcp.json (gitnexus@ launch arg, x10) + * - gitnexus-factory-plugin/.factory-plugin/plugin.json (top-level version) + * - gitnexus-factory-plugin/mcp.json (gitnexus@ launch arg) + * - .factory-plugin/marketplace.json (plugins[gitnexus]) * * Modes: * node scripts/sync-plugin-manifests.mjs rewrite stale surfaces @@ -54,6 +57,16 @@ const MANIFEST_SURFACES = [ file: `gitnexus-claude-plugin/skills/${name}/mcp.json`, kind: 'mcp', })), + // The Factory plugin's manifest is also its pin source at runtime: the + // PostToolUse hook reads this version to build its `npx -y gitnexus@` + // fallback, so stamping it here keeps the hook and the MCP entry on the same + // released CLI. + { file: 'gitnexus-factory-plugin/.factory-plugin/plugin.json', kind: 'plugin' }, + { file: 'gitnexus-factory-plugin/mcp.json', kind: 'mcp' }, + // Droid reads .factory-plugin/marketplace.json before .claude-plugin's, so + // this entry is what makes `droid plugin install` deliver the Factory plugin + // (Execute matcher, pinned mcp.json) instead of the translated Claude one. + { file: '.factory-plugin/marketplace.json', kind: 'marketplace' }, ]; const PLUGIN_NAME = 'gitnexus'; diff --git a/gitnexus/src/cli/editor-targets.ts b/gitnexus/src/cli/editor-targets.ts index a1bf1841b..212a4c400 100644 --- a/gitnexus/src/cli/editor-targets.ts +++ b/gitnexus/src/cli/editor-targets.ts @@ -26,7 +26,8 @@ export type EditorId = | 'opencode' | 'codebuddy' | 'qoder' - | 'codex'; + | 'codex' + | 'droid'; /** An editor whose MCP config is a JSONC document (server keyed by name). */ export interface McpJsoncTarget { @@ -151,6 +152,15 @@ export function getEditorTargets(home: string = os.homedir()): EditorTargets { file: path.join(home, '.qoder.json'), keyPath: ['mcpServers', 'gitnexus'], }, + { + id: 'droid', + label: 'Factory Droid', + // Factory's user-scope MCP config (https://docs.factory.ai/cli/configuration/mcp). + // Same `mcpServers` object shape as Cursor/Claude; user config takes + // precedence over project-level .factory/mcp.json. + file: path.join(home, '.factory', 'mcp.json'), + keyPath: ['mcpServers', 'gitnexus'], + }, ]; const codex: CodexMcpTarget = { @@ -175,6 +185,9 @@ export function getEditorTargets(home: string = os.homedir()): EditorTargets { { id: 'qoder', label: 'Qoder', dir: path.join(home, '.qoder', 'skills') }, // Codex reads skills from ~/.agents/skills (not ~/.codex). { id: 'codex', label: 'Codex', dir: path.join(home, '.agents', 'skills') }, + // Factory Droid reads user-scope skills from ~/.factory/skills/{name}/SKILL.md + // (https://docs.factory.ai/cli/configuration/skills). + { id: 'droid', label: 'Factory Droid', dir: path.join(home, '.factory', 'skills') }, ]; const hooks: HookTarget[] = [ diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index 32444ceac..aa80a1f44 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -198,7 +198,7 @@ export const en = { 'help.option.help': 'display help for command', 'help.option.version': 'output the version number', 'help.command.setup.description': - 'One-time setup: configure MCP for Cursor, Claude Code, Antigravity, OpenCode, CodeBuddy, Qoder, Codex', + 'One-time setup: configure MCP for Cursor, Claude Code, Antigravity, OpenCode, CodeBuddy, Qoder, Codex, Factory Droid', 'help.command.uninstall.description': 'Reverse `setup`: remove GitNexus MCP entries, skills, and hooks from all detected editors', 'help.command.autoSync.description': diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index 33097d0b7..3b53b151f 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -188,7 +188,7 @@ export const zhCN = { 'help.option.help': '显示命令帮助', 'help.option.version': '输出版本号', 'help.command.setup.description': - '一次性设置:为 Cursor、Claude Code、Antigravity、OpenCode、CodeBuddy、Qoder、Codex 配置 MCP', + '一次性设置:为 Cursor、Claude Code、Antigravity、OpenCode、CodeBuddy、Qoder、Codex、Factory Droid 配置 MCP', 'help.command.uninstall.description': '撤销 `setup`:从所有检测到的编辑器中移除 GitNexus 的 MCP 配置、技能和钩子', 'help.command.autoSync.description': diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index f3fa5d8fa..a8eab181c 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -28,7 +28,7 @@ program.name('gitnexus').description('GitNexus local CLI and MCP server').versio program .command('setup') .description( - 'One-time setup: configure MCP for Cursor, Claude Code, Antigravity, OpenCode, CodeBuddy, Qoder, Codex', + 'One-time setup: configure MCP for Cursor, Claude Code, Antigravity, OpenCode, CodeBuddy, Qoder, Codex, Factory Droid', ) .option( '-c, --coding-agent ', diff --git a/gitnexus/src/cli/setup.ts b/gitnexus/src/cli/setup.ts index 3c74877e8..f03c58aa3 100644 --- a/gitnexus/src/cli/setup.ts +++ b/gitnexus/src/cli/setup.ts @@ -96,6 +96,7 @@ const CODING_AGENT_IDS = { codebuddy: 'codebuddy', qoder: 'qoder', codex: 'codex', + droid: 'droid', } as const satisfies Record; const SUPPORTED_CODING_AGENTS = Object.values(CODING_AGENT_IDS); @@ -954,6 +955,59 @@ async function installQoderSkills(result: SetupResult): Promise { } } +/** + * Configure the GitNexus MCP server for Factory Droid. + * + * Factory stores user-scope MCP config in ~/.factory/mcp.json using the same + * `{ mcpServers: { : {...} } }` JSONC shape as Cursor/Claude + * (https://docs.factory.ai/cli/configuration/mcp), so we reuse mergeJsoncFile + * rather than shelling out to `droid mcp add` — that keeps setup working even + * when the `droid` binary isn't on PATH. + */ +async function setupDroid(result: SetupResult): Promise { + const factoryDir = path.join(os.homedir(), '.factory'); + if (!(await dirExists(factoryDir))) { + result.skipped.push('Factory Droid (not installed)'); + return; + } + + const { file: mcpPath, keyPath } = mcpTarget('droid'); + try { + const ok = await mergeJsoncFile(mcpPath, keyPath, getMcpEntry()); + if (ok) { + result.configured.push('Factory Droid'); + } else { + result.errors.push( + 'Factory Droid: mcp.json is corrupt — skipping to preserve existing content', + ); + } + } catch (err: any) { + result.errors.push(`Factory Droid: ${err.message}`); + } +} + +/** + * Install global Factory Droid skills to ~/.factory/skills/{name}/SKILL.md + * (https://docs.factory.ai/cli/configuration/skills — same SKILL.md layout as + * Claude Code). + */ +async function installDroidSkills(result: SetupResult): Promise { + const factoryDir = path.join(os.homedir(), '.factory'); + if (!(await dirExists(factoryDir))) return; + + const skillsDir = skillTarget('droid').dir; + try { + const installed = await installSkillsTo(skillsDir); + if (installed.length > 0) { + result.configured.push( + `Factory Droid skills (${installed.length} skills → ~/.factory/skills/)`, + ); + } + } catch (err: any) { + result.errors.push(`Factory Droid skills: ${err.message}`); + } +} + /** * Build a TOML section for Codex MCP config (~/.codex/config.toml). */ @@ -1255,6 +1309,7 @@ export const setupCommand = async (options?: { codingAgent?: string[] | string } if (selected.has('codebuddy')) await setupCodeBuddy(result); if (selected.has('qoder')) await setupQoder(result); if (selected.has('codex')) await setupCodex(result); + if (selected.has('droid')) await setupDroid(result); // Install global skills for platforms that support them if (selected.has('claude')) { @@ -1273,6 +1328,10 @@ export const setupCommand = async (options?: { codingAgent?: string[] | string } await installCodexSkills(result); await installClaudeSchemaHooks(result, 'codex'); } + // MCP + skills only. Factory hooks (Execute matcher, PostToolUse) ship in the + // standalone gitnexus-factory-plugin instead of the setup path — add a droid + // hook target + adapter here if setup-installed hooks are wanted. + if (selected.has('droid')) await installDroidSkills(result); // Print results if (result.configured.length > 0) { diff --git a/gitnexus/test/integration/setup-uninstall-roundtrip.test.ts b/gitnexus/test/integration/setup-uninstall-roundtrip.test.ts index b72774372..100b1007a 100644 --- a/gitnexus/test/integration/setup-uninstall-roundtrip.test.ts +++ b/gitnexus/test/integration/setup-uninstall-roundtrip.test.ts @@ -82,7 +82,7 @@ describe('setup → uninstall round-trip', () => { process.env.USERPROFILE = tempHome; // Mark every editor as "installed" so setup configures all of them. - for (const dir of ['.cursor', '.claude', '.codex', '.codebuddy', '.qoder']) { + for (const dir of ['.cursor', '.claude', '.codex', '.codebuddy', '.qoder', '.factory']) { await fs.mkdir(path.join(tempHome, dir), { recursive: true }); } await fs.mkdir(path.join(tempHome, '.gemini', 'antigravity'), { recursive: true }); diff --git a/gitnexus/test/unit/factory-plugin.test.ts b/gitnexus/test/unit/factory-plugin.test.ts new file mode 100644 index 000000000..7a9609019 --- /dev/null +++ b/gitnexus/test/unit/factory-plugin.test.ts @@ -0,0 +1,809 @@ +/** + * Tests: GitNexus Factory AI (Droid) plugin + * + * Covers the standalone `gitnexus-factory-plugin/` used by `droid plugin + * install`: + * - manifest + hook wiring (plugin.json / mcp.json / hooks.json) + * - the PostToolUse search-augment hook's guard reuse and early-exit behavior + * - a drift guard proving the bundled guard modules are byte-identical to the + * canonical Claude-adapter copies (so a fix to one can't silently skip the + * other) + * + * The augment fan-out guard (acquireHookSlot) and the LadybugDB owner probe are + * the exact modules the Claude/Codex adapter ships; their internals are covered + * by hooks.test.ts and hook-db-lock-probe.test.ts. Here we assert the Factory + * hook WIRES them and behaves correctly on the Factory-specific paths. + */ +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { spawnSync } from 'child_process'; +import { createRequire } from 'module'; +import fs from 'fs'; +import path from 'path'; +import os from 'os'; +import { + runHook, + parseHookOutput, + createHookToolDir, + hookEnv, +} from '../utils/hook-test-helpers.js'; + +const REPO_ROOT = path.resolve(__dirname, '..', '..', '..'); +const PLUGIN_DIR = path.join(REPO_ROOT, 'gitnexus-factory-plugin'); +const HOOK = path.join(PLUGIN_DIR, 'hooks', 'gitnexus-hook.js'); +const HOOKS_JSON = path.join(PLUGIN_DIR, 'hooks', 'hooks.json'); +const PLUGIN_JSON = path.join(PLUGIN_DIR, '.factory-plugin', 'plugin.json'); +const MCP_JSON = path.join(PLUGIN_DIR, 'mcp.json'); +const CLAUDE_HOOKS = path.join(REPO_ROOT, 'gitnexus-claude-plugin', 'hooks'); + +// Guard and repo-lookup modules bundled into the Factory plugin, kept +// byte-identical to the canonical Claude-adapter copies. +const BUNDLED_GUARDS = [ + 'hook-lock.js', + 'hook-db-lock-probe.cjs', + 'win-rm-list-json.ps1', + 'registry-query.cjs', +] as const; + +// Empty GITNEXUS_HOME and no storage overrides, so behavior tests never pick +// up the developer's real registry or storage config. +// Unset means absent: an empty-string override is set-but-invalid (the hook, +// like the CLI, then resolves no storage), so the keys are removed, not blanked. +function isolatedEnv(binDir: string, home: string) { + const env: NodeJS.ProcessEnv = { ...hookEnv(binDir), GITNEXUS_HOME: home }; + delete env.GITNEXUS_STORAGE_PATH; + delete env.GITNEXUS_STORAGE_ROOT; + return env; +} + +/** Source text of top-level `function (` through its closing brace. */ +function fnSource(file: string, name: string): string { + const src = fs.readFileSync(file, 'utf-8'); + const start = src.indexOf(`function ${name}(`); + const end = start < 0 ? -1 : src.indexOf('\n}\n', start); + if (end < 0) throw new Error(`${name} not found in ${file}`); + return src.slice(start, end + 3); +} + +const require_ = createRequire(import.meta.url); +const { parseRgGrepPattern } = require_(HOOK) as { + parseRgGrepPattern: (command: string) => string | null; +}; + +// ─── Manifest / file presence ─────────────────────────────────────── + +describe('Factory plugin files', () => { + it('ships the hook, its guards, and both manifests', () => { + for (const p of [ + HOOK, + HOOKS_JSON, + PLUGIN_JSON, + MCP_JSON, + ...BUNDLED_GUARDS.map((f) => path.join(PLUGIN_DIR, 'hooks', f)), + ]) { + expect(fs.existsSync(p), `${p} should exist`).toBe(true); + } + }); + + it('.factory-plugin holds only plugin.json (Factory manifest contract)', () => { + expect(fs.readdirSync(path.join(PLUGIN_DIR, '.factory-plugin'))).toEqual(['plugin.json']); + }); +}); + +// ─── Marketplace wiring ───────────────────────────────────────────── + +// Droid reads .factory-plugin/marketplace.json before .claude-plugin's, so this +// is what makes `droid plugin install` deliver this Factory plugin instead of +// the translated Claude one (Bash matcher, gitnexus@latest MCP). +describe('Factory marketplace wiring', () => { + const marketplace = JSON.parse( + fs.readFileSync(path.join(REPO_ROOT, '.factory-plugin', 'marketplace.json'), 'utf-8'), + ); + + it('exposes a single gitnexus plugin sourced from ./gitnexus-factory-plugin', () => { + const entries = marketplace.plugins.filter((p: { name: string }) => p.name === 'gitnexus'); + expect(entries).toHaveLength(1); + expect(entries[0].source).toBe('./gitnexus-factory-plugin'); + }); +}); + +// ─── Drift guard: bundled guards === canonical Claude copies ───────── + +describe('Factory plugin bundled guards stay in lockstep with the Claude adapter', () => { + for (const f of BUNDLED_GUARDS) { + it(`${f} is byte-identical to the canonical copy`, () => { + const bundled = fs.readFileSync(path.join(PLUGIN_DIR, 'hooks', f)); + const canonical = fs.readFileSync(path.join(CLAUDE_HOOKS, f)); + expect(bundled.equals(canonical)).toBe(true); + }); + } +}); + +// ─── plugin.json ──────────────────────────────────────────────────── + +describe('Factory plugin.json', () => { + const manifest = JSON.parse(fs.readFileSync(PLUGIN_JSON, 'utf-8')); + + it('is named gitnexus', () => { + expect(manifest.name).toBe('gitnexus'); + }); + + it('version matches gitnexus/package.json (single source of truth)', () => { + const pkg = JSON.parse( + fs.readFileSync(path.join(REPO_ROOT, 'gitnexus', 'package.json'), 'utf-8'), + ); + expect(manifest.version).toBe(pkg.version); + }); +}); + +// ─── mcp.json ─────────────────────────────────────────────────────── + +describe('Factory mcp.json', () => { + const mcp = JSON.parse(fs.readFileSync(MCP_JSON, 'utf-8')); + + it('registers the gitnexus MCP server under mcpServers', () => { + expect(mcp.mcpServers?.gitnexus?.command).toBe('npx'); + expect(mcp.mcpServers.gitnexus.args).toContain('mcp'); + }); + + // Executed state, not quickstart docs: `@latest` would let a future registry + // upload run on MCP connect without a plugin revision. + it('pins the CLI to the released version instead of a mutable tag', () => { + const pkg = JSON.parse( + fs.readFileSync(path.join(REPO_ROOT, 'gitnexus', 'package.json'), 'utf-8'), + ); + expect(mcp.mcpServers.gitnexus.args).toContain(`gitnexus@${pkg.version}`); + expect(JSON.stringify(mcp)).not.toContain('gitnexus@latest'); + }); +}); + +// ─── hooks.json wiring ────────────────────────────────────────────── + +describe('Factory hooks.json wiring', () => { + const manifest = JSON.parse(fs.readFileSync(HOOKS_JSON, 'utf-8')); + const entry = manifest.PostToolUse[0]; + + it('registers a PostToolUse hook', () => { + expect(Array.isArray(manifest.PostToolUse)).toBe(true); + }); + + it('matches Factory search tools (Grep, Glob, Execute — not Bash)', () => { + expect(entry.matcher).toBe('Grep|Glob|Execute'); + expect(entry.matcher).not.toMatch(/\bBash\b/); + }); + + it('invokes the hook via the quoted ${DROID_PLUGIN_ROOT} plugin-root path', () => { + // The path must be quoted so a plugin root containing spaces (e.g. + // `C:\Users\First Last\...`) stays one argv word — mirrors the Claude plugin. + const command: string = entry.hooks[0].command; + expect(command).toBe('node "${DROID_PLUGIN_ROOT}/hooks/gitnexus-hook.js"'); + }); + + it('declares timeout in seconds (not milliseconds)', () => { + const timeout: number = entry.hooks[0].timeout; + expect(typeof timeout).toBe('number'); + expect(timeout).toBeGreaterThan(0); + expect(timeout).toBeLessThan(120); + }); +}); + +// ─── Source regressions ───────────────────────────────────────────── + +describe('Factory hook source regressions', () => { + const source = fs.readFileSync(HOOK, 'utf-8'); + + it('wires the augment fan-out guard (acquireHookSlot + finally release)', () => { + expect(source).toContain("require('./hook-lock.js')"); + expect(source).toContain('acquireHookSlot('); + expect(source).toMatch(/finally\s*\{[^}]*release\(\)/s); + }); + + it('wires the LadybugDB owner probe before running augment', () => { + expect(source).toContain("require('./hook-db-lock-probe.cjs')"); + expect(source).toContain('hasGitNexusDbLockedByGitNexusServer('); + }); + + it('never passes shell: true / shell: isWin to spawnSync (injection risk)', () => { + const codeLines = source + .split('\n') + .map((line) => line.trim()) + .filter((t) => !t.startsWith('//') && !t.startsWith('*')); + for (const t of codeLines) { + expect(/shell:\s*(true|isWin)/.test(t), `injection risk: ${t}`).toBe(false); + } + }); + + it('invokes npx.cmd directly on Windows instead of a shell', () => { + expect(source).toContain('npx.cmd'); + }); + + // The npx fallback runs on ordinary tool calls whenever the CLI is not on + // PATH, so an unpinned ref would execute whatever currently owns the tag. + it('pins the npx fallback to the manifest version, never a mutable tag', () => { + expect(source).not.toContain('gitnexus@latest'); + expect(source).toContain("require('../.factory-plugin/plugin.json')"); + expect(source).toContain('`gitnexus@${PINNED_VERSION}`'); + }); + + // Windows regression: Node refuses to spawn the .cmd launcher shims without a + // shell (CVE-2024-27980), so `node ` is the only branch that runs + // there. Same escape hatch the Claude adapter honors. + it('prefers GITNEXUS_HOOK_CLI_PATH via process.execPath', () => { + expect(source).toContain('GITNEXUS_HOOK_CLI_PATH'); + expect(source).toMatch(/spawnAugment\(\s*process\.execPath/); + expect(source).toContain('spawnSync(file, fileArgs, spawnOpts)'); + }); + + // #2163: the augment child runs under the bundled probe's timeout guard, + // TERM-first for direct children, group SIGKILL for the npx grandchild. + it('wraps the augment child in the resolved Unix timeout guard', () => { + expect(source).toContain('resolveUnixGuardTimeout()'); + expect(source).toMatch(/groupKill \? \['-s', 'KILL'\] : \[\]/); + expect(source).toMatch(/`gitnexus@\$\{PINNED_VERSION\}`, \.\.\.args\],\s*true,/); + }); + + it('passes the pattern after the -- end-of-options marker', () => { + expect(source).toMatch(/'augment',\s*'--',\s*pattern/); + }); + + it('validates cwd is absolute and resolves the repo via the shared registry lookup', () => { + expect(source).toMatch(/path\.isAbsolute\(cwd\)/); + expect(source).toContain("require('./registry-query.cjs')"); + expect(source).toContain('resolveHookRepo(cwd)'); + }); + + // #3060: indexes can live outside the repo, so the slot and the DB-owner + // probe must target the resolved storage, not a hardcoded `/.gitnexus`. + it('keys the slot and DB-owner probe on the resolved storage', () => { + expect(source).toContain('acquireHookSlot(repo.storagePath)'); + expect(source).toContain('hasGitNexusDbLockedByGitNexusServer(repo.lbugPath'); + }); + + it('emits Factory-shape hookSpecificOutput.additionalContext', () => { + expect(source).toContain('hookSpecificOutput'); + expect(source).toContain('additionalContext'); + }); + + it('rejects patterns shorter than 3 chars', () => { + expect(source).toMatch(/length\s*<\s*3/); + }); +}); + +// ─── Execute pattern parsing (shares the Cursor adapter's #2938 matrix) ─ + +describe('Factory Execute pattern parser', () => { + it.each([ + ['rg "User Service" src/', 'User Service'], + ["grep 'error boundary' -- src/", 'error boundary'], + ['rg User\\ Service src/', 'User Service'], + [String.raw`rg "C:\Users" src/`, String.raw`C:\Users`], + ['rg -e "User Service" src/', 'User Service'], + ['rg --regexp=UserService src/', 'UserService'], + ['grep -eUserService src/', 'UserService'], + ['/usr/bin/rg -- "User Service" src/', 'User Service'], + ['rg -- -error src/', '-error'], + ['rg "validateUser"', 'validateUser'], + ['rg -t ts UserService src/', 'UserService'], + ['rg --glob "*.ts" UserService', 'UserService'], + ['rg ab src/', null], + // rg/grep only counts at command position, not as an argument. + ['echo rg UserService', null], + // -f reads patterns from a file; later positionals are paths. + ['rg -f patterns.txt src/', null], + ['grep --file=patterns.txt src/', null], + ])('extracts %j from %j', (command, expected) => { + expect(parseRgGrepPattern(command)).toBe(expected); + }); + + // Same parser as the Cursor adapter; fails if either copy drifts. + it.each(['tokenizeShellWords', 'parseRgGrepPattern'])( + '%s is identical to the Cursor adapter', + (name) => { + const cursor = path.join( + REPO_ROOT, + 'gitnexus-cursor-integration', + 'hooks', + 'gitnexus-hook.cjs', + ); + expect(fnSource(HOOK, name)).toBe(fnSource(cursor, name)); + }, + ); +}); + +// Same augment-stderr filter as the Claude adapter; fails if either copy drifts. +describe('Factory augment stderr filter', () => { + it.each(['extractAugmentContext', 'isDebugEnabled'])( + '%s is identical to the Claude adapter', + (name) => { + const claude = path.join(CLAUDE_HOOKS, 'gitnexus-hook.js'); + expect(fnSource(HOOK, name)).toBe(fnSource(claude, name)); + }, + ); +}); + +// ─── Behavior: early-exit paths (no augment spawned) ──────────────── + +describe('Factory hook behavior — early exits', () => { + let tmpDir: string; + + beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-early-')); + }); + afterAll(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + + it('exits cleanly on empty stdin', () => { + const r = spawnSync(process.execPath, [HOOK], { + input: '', + encoding: 'utf-8', + timeout: 10000, + stdio: ['pipe', 'pipe', 'pipe'], + }); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + }); + + it('exits cleanly on invalid JSON stdin', () => { + const r = spawnSync(process.execPath, [HOOK], { + input: 'not json', + encoding: 'utf-8', + timeout: 10000, + stdio: ['pipe', 'pipe', 'pipe'], + }); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + }); + + it('ignores non-PostToolUse events', () => { + const r = runHook(HOOK, { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + }); + + it('produces no output when cwd is relative', () => { + const r = runHook(HOOK, { + hook_event_name: 'PostToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: 'relative/path', + }); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + }); + + it('produces no output when cwd has no .gitnexus index', () => { + const r = runHook(HOOK, { + hook_event_name: 'PostToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + }); + + it('produces no output for unmatched tools or Execute without rg/grep', () => { + for (const input of [ + { tool_name: 'MadeUpTool', tool_input: { foo: 'bar' } }, + { tool_name: 'Execute', tool_input: { command: 'ls -la' } }, + { tool_name: 'Grep', tool_input: { pattern: 'is' } }, // < 3 chars + ]) { + const r = runHook(HOOK, { hook_event_name: 'PostToolUse', cwd: tmpDir, ...input }); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + } + }); +}); + +// ─── Behavior: happy path (augment via a fake gitnexus on PATH) ───── + +describe('Factory hook behavior — augment', () => { + let repoDir: string; + let binDir: string; + let home: string; + + beforeAll(() => { + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-repo-')); + // Repo-local index metadata → resolved as a local owned index; no `lbug` + // file → the DB-owner probe short-circuits false and augment runs. + fs.mkdirSync(path.join(repoDir, '.gitnexus'), { recursive: true }); + fs.writeFileSync(path.join(repoDir, '.gitnexus', 'gitnexus.json'), '{}'); + binDir = createHookToolDir({ gitnexusStderr: '[GitNexus] graph context for validateUser' }); + home = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-home-')); + }); + afterAll(() => { + fs.rmSync(repoDir, { recursive: true, force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + fs.rmSync(home, { recursive: true, force: true }); + }); + + it('emits augment stderr as additionalContext for a Grep search', () => { + const r = runHook( + HOOK, + { + hook_event_name: 'PostToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: repoDir, + }, + undefined, + { env: isolatedEnv(binDir, home) }, + ); + expect(r.status).toBe(0); + const out = parseHookOutput(r.stdout); + expect(out?.hookEventName).toBe('PostToolUse'); + expect(out?.additionalContext).toContain('graph context for validateUser'); + }); + + it('augments against an external index registered in GITNEXUS_HOME (#3060)', () => { + const extRepo = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-extrepo-')); + const storage = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-storage-')); + const extHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-exthome-')); + try { + // No repo-local .gitnexus: only the registry row points at the index. + fs.writeFileSync( + path.join(storage, 'gitnexus.json'), + JSON.stringify({ repoPath: extRepo, storagePath: storage }), + ); + fs.writeFileSync( + path.join(extHome, 'registry.json'), + JSON.stringify([{ name: 'ext', path: extRepo, storagePath: storage }]), + ); + + const r = runHook( + HOOK, + { + hook_event_name: 'PostToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: extRepo, + }, + undefined, + { env: isolatedEnv(binDir, extHome) }, + ); + expect(r.status).toBe(0); + expect(parseHookOutput(r.stdout)?.additionalContext).toContain( + 'graph context for validateUser', + ); + // The fan-out slot lives in the resolved storage, not the repo. + expect(fs.existsSync(path.join(storage, '.hook-locks'))).toBe(true); + expect(fs.existsSync(path.join(extRepo, '.gitnexus'))).toBe(false); + } finally { + for (const d of [extRepo, storage, extHome]) fs.rmSync(d, { recursive: true, force: true }); + } + }); +}); + +// ─── Behavior: fan-out guard skips when all slots are held ────────── + +describe('Factory hook behavior — augment fan-out guard', () => { + it('exits silently when all MAX_INFLIGHT slots hold live pids', async () => { + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-slots-')); + const lockDir = path.join(repoDir, '.gitnexus', '.hook-locks'); + fs.mkdirSync(lockDir, { recursive: true }); + // Index metadata so the hook resolves the repo and reaches the slot guard, + // rather than exiting early for having no index. + fs.writeFileSync(path.join(repoDir, '.gitnexus', 'gitnexus.json'), '{}'); + const binDir = createHookToolDir({ gitnexusStderr: 'should never be emitted' }); + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-home-')); + + const { spawn } = await import('child_process'); + const sleepers = [0, 1, 2].map(() => + spawn(process.execPath, ['-e', 'setTimeout(()=>{},60000)'], { stdio: 'ignore' }), + ); + try { + sleepers.forEach((s, i) => + fs.writeFileSync(path.join(lockDir, `slot-${i}.lock`), String(s.pid)), + ); + + const r = runHook( + HOOK, + { + hook_event_name: 'PostToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: repoDir, + }, + undefined, + { env: isolatedEnv(binDir, home) }, + ); + + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + } finally { + for (const s of sleepers) { + try { + s.kill(); + } catch { + /* ignore */ + } + } + fs.rmSync(repoDir, { recursive: true, force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + fs.rmSync(home, { recursive: true, force: true }); + } + }); +}); + +// ─── Behavior: PATH tier, no GITNEXUS_HOOK_CLI_PATH ───────────────── +// +// PATH holds only the fake launchers written here (`#!/bin/sh` shebangs resolve +// by absolute path, so no other PATH entry is needed), which keeps any real +// `gitnexus` or `npx` on the host out of the result. + +describe.skipIf(process.platform === 'win32')('Factory hook behavior — PATH augment tier', () => { + let repoDir: string; + let toolsDir: string; + let home: string; + + beforeAll(() => { + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-pathrepo-')); + fs.mkdirSync(path.join(repoDir, '.gitnexus'), { recursive: true }); + fs.writeFileSync(path.join(repoDir, '.gitnexus', 'gitnexus.json'), '{}'); + toolsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-pathtools-')); + home = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-pathhome-')); + }); + afterAll(() => { + for (const d of [repoDir, toolsDir, home]) fs.rmSync(d, { recursive: true, force: true }); + }); + + function makeBinDir(name: string, scripts: Record): string { + const dir = path.join(toolsDir, name); + fs.mkdirSync(dir); + for (const [file, body] of Object.entries(scripts)) { + fs.writeFileSync(path.join(dir, file), `#!/bin/sh\n${body}\n`, { mode: 0o755 }); + } + return dir; + } + + // Default: the guard disabled, so the host's coreutils cannot change which arm + // runs; the guarded arm is pinned explicitly below with a logging fake guard. + function runGrepHook(binDir: string, timeoutPath = 'disabled') { + return runHook( + HOOK, + { + hook_event_name: 'PostToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: repoDir, + }, + undefined, + { + env: { + ...isolatedEnv(binDir, home), + PATH: binDir, + GITNEXUS_HOOK_CLI_PATH: '', + GITNEXUS_HOOK_TIMEOUT_PATH: timeoutPath, + }, + }, + ); + } + + /** + * A coreutils-shaped `timeout` stand-in: appends its argv to `log`, drops the + * `-s SIG` / `-k N` options and the duration, then execs the command, so it + * passes the probe's `-k 1 1 /bin/sh -c 'exit 42'` self-test. + */ + function writeLoggingGuard(name: string): { guard: string; log: string } { + const log = path.join(toolsDir, `${name}.log`); + const guard = path.join(toolsDir, `${name}-guard`); + fs.writeFileSync( + guard, + `#!/bin/sh\necho "$*" >> '${log}'\n` + + `while [ "$1" = -s ] || [ "$1" = -k ]; do shift 2; done\nshift\nexec "$@"\n`, + { mode: 0o755 }, + ); + return { guard, log }; + } + + function augmentGuardCalls(log: string): string[] { + return fs + .readFileSync(log, 'utf-8') + .split('\n') + .filter((line) => line.includes('augment')); + } + + for (const guarded of [false, true]) { + const arm = guarded ? 'guarded' : 'unguarded'; + + it(`${arm}: does not re-run augment via npx when the PATH binary finds no match`, () => { + const marker = path.join(toolsDir, `npx-called-${arm}`); + const binDir = makeBinDir(`no-match-${arm}`, { + gitnexus: 'exit 0', + npx: `: > '${marker}'\nprintf '[GitNexus] from npx' >&2`, + }); + const r = runGrepHook( + binDir, + guarded ? writeLoggingGuard(`no-match-${arm}`).guard : 'disabled', + ); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + expect(fs.existsSync(marker)).toBe(false); + }); + + it(`${arm}: falls through to npx when no gitnexus launcher is on PATH`, () => { + const binDir = makeBinDir(`npx-only-${arm}`, { + npx: "printf '[GitNexus] graph context via npx' >&2", + }); + const r = runGrepHook( + binDir, + guarded ? writeLoggingGuard(`npx-only-${arm}`).guard : 'disabled', + ); + expect(r.status).toBe(0); + expect(parseHookOutput(r.stdout)?.additionalContext).toContain('graph context via npx'); + }); + } + + // #2163: direct children get TERM-first `-k 1`; the budget is ceil(8000/1000)+1. + it('runs the PATH binary under the timeout guard (TERM-first)', () => { + const binDir = makeBinDir('guarded-path', { + gitnexus: "printf '[GitNexus] graph context via guarded PATH' >&2", + }); + const { guard, log } = writeLoggingGuard('guarded-path'); + const r = runGrepHook(binDir, guard); + expect(parseHookOutput(r.stdout)?.additionalContext).toContain('via guarded PATH'); + expect(augmentGuardCalls(log)).toEqual([ + `-k 1 9 ${path.join(binDir, 'gitnexus')} augment -- validateUser`, + ]); + }); + + // The CLI is npx's child (the guard's grandchild), so npx needs `-s KILL`. + it('runs the npx fallback under the group-SIGKILL timeout guard', () => { + const binDir = makeBinDir('guarded-npx', { + npx: "printf '[GitNexus] graph context via guarded npx' >&2", + }); + const { guard, log } = writeLoggingGuard('guarded-npx'); + const { version } = JSON.parse(fs.readFileSync(PLUGIN_JSON, 'utf-8')) as { version: string }; + const r = runGrepHook(binDir, guard); + expect(parseHookOutput(r.stdout)?.additionalContext).toContain('via guarded npx'); + expect(augmentGuardCalls(log)).toEqual([ + `-s KILL -k 1 9 npx -y gitnexus@${version} augment -- validateUser`, + ]); + }); + + it('drops launcher noise ahead of the [GitNexus] block', () => { + const binDir = makeBinDir('noisy-match', { + gitnexus: + "printf 'npm warn config production\\n(node:42) ExperimentalWarning: noise\\n[GitNexus] graph context for validateUser\\n' >&2", + }); + const r = runGrepHook(binDir); + expect(r.status).toBe(0); + const context = parseHookOutput(r.stdout)?.additionalContext; + expect(context).toBe('[GitNexus] graph context for validateUser'); + expect(context).not.toContain('npm warn'); + expect(context).not.toContain('ExperimentalWarning'); + }); + + it('emits nothing when augment stderr is only noise (no [GitNexus] marker)', () => { + const binDir = makeBinDir('noise-only', { + gitnexus: "printf 'npm warn config production\\n(node:42) ExperimentalWarning: noise\\n' >&2", + }); + const r = runGrepHook(binDir); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe(''); + }); +}); + +// ─── Behavior: a SIGKILLed hook cannot strand the augment CLI (#2163) ── +// +// Real coreutils `timeout` (the path under test): the fake CLI is SIGTERM-immune +// and sleeps 30s, so only the guard can end it once the hook is gone. Direct +// tier: TERM at 9s (= ceil(8000/1000)+1) is ignored, the `-k 1` KILL lands at +// 10s. npx tier: `-s KILL` group-kills the npx → CLI chain at 9s. With the guard +// wrap reverted nothing reaps the CLI and the poll times out. + +const factoryProbe = require_(path.join(PLUGIN_DIR, 'hooks', 'hook-db-lock-probe.cjs')) as { + resolveUnixGuardTimeout: () => string | null; +}; + +describe.skipIf(process.platform !== 'linux')( + 'Factory hook behavior — orphaned augment CLI is reaped by the timeout guard (#2163)', + () => { + let repoDir: string; + let home: string; + + beforeAll(() => { + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-reaprepo-')); + fs.mkdirSync(path.join(repoDir, '.gitnexus'), { recursive: true }); + fs.writeFileSync(path.join(repoDir, '.gitnexus', 'gitnexus.json'), '{}'); + home = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-factory-reaphome-')); + }); + afterAll(() => { + for (const d of [repoDir, home]) fs.rmSync(d, { recursive: true, force: true }); + }); + + const isAlive = (pid: number, binDir: string): boolean => { + try { + process.kill(pid, 0); + // PID-reuse guard: alive only while the cmdline is still our fake CLI. + return fs.readFileSync(`/proc/${pid}/cmdline`, 'utf-8').includes(binDir); + } catch { + return false; + } + }; + + async function killHookMidAugment(tier: 'direct' | 'npx'): Promise { + const { spawn } = await import('child_process'); + const pidFile = path.join(os.tmpdir(), `gn-factory-clipid-${process.pid}-${tier}`); + fs.rmSync(pidFile, { force: true }); + const binDir = createHookToolDir({ + gitnexusPidFile: pidFile, + gitnexusSleepMs: 30000, + gitnexusIgnoreSigterm: true, + }); + // npx tier: no gitnexus on PATH and no CLI-path override; the fake npx is + // a shell that waits on the CLI, so the CLI is the guard's grandchild. + fs.rmSync(path.join(binDir, 'gitnexus')); + fs.writeFileSync( + path.join(binDir, 'npx'), + `#!/bin/sh\n'${process.execPath}' '${path.join(binDir, 'gitnexus-cli.js')}'\n`, + { mode: 0o755 }, + ); + const tierEnv = + tier === 'npx' ? { PATH: binDir, GITNEXUS_HOOK_CLI_PATH: '' } : { PATH: binDir }; + let cliPid = 0; + const hook = spawn(process.execPath, [HOOK], { + stdio: ['pipe', 'ignore', 'ignore'], + env: { ...isolatedEnv(binDir, home), GITNEXUS_HOOK_TIMEOUT_PATH: '', ...tierEnv }, + }); + try { + hook.stdin?.end( + JSON.stringify({ + hook_event_name: 'PostToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: repoDir, + }), + ); + const spawnDeadline = Date.now() + 8000; + while (cliPid === 0 && Date.now() < spawnDeadline) { + cliPid = + Number.parseInt(fs.existsSync(pidFile) ? fs.readFileSync(pidFile, 'utf-8') : '', 10) || + 0; + await new Promise((r) => setTimeout(r, 10)); + } + expect(cliPid).toBeGreaterThan(0); + hook.kill('SIGKILL'); + + const reapDeadline = Date.now() + 14000; + let alive = isAlive(cliPid, binDir); + while (alive && Date.now() < reapDeadline) { + await new Promise((r) => setTimeout(r, 100)); + alive = isAlive(cliPid, binDir); + } + return alive; + } finally { + hook.kill('SIGKILL'); + for (const pid of [cliPid].filter((p) => p > 0 && isAlive(p, binDir))) { + process.kill(pid, 'SIGKILL'); + } + fs.rmSync(path.join(repoDir, '.gitnexus', '.hook-locks'), { recursive: true, force: true }); + fs.rmSync(pidFile, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + } + + it('host exposes a self-testing timeout guard (precondition)', () => { + vi.stubEnv('GITNEXUS_HOOK_TIMEOUT_PATH', ''); + try { + expect( + factoryProbe.resolveUnixGuardTimeout(), + 'install coreutils `timeout`', + ).not.toBeNull(); + } finally { + vi.unstubAllEnvs(); + } + }); + + it('direct tier: SIGKILLed hook leaves no SIGTERM-immune CLI child', async () => { + expect(await killHookMidAugment('direct')).toBe(false); + }, 30000); + + it('npx tier: SIGKILLed hook leaves no SIGTERM-immune CLI grandchild', async () => { + expect(await killHookMidAugment('npx')).toBe(false); + }, 30000); + }, +); diff --git a/gitnexus/test/unit/hook-db-lock-probe.test.ts b/gitnexus/test/unit/hook-db-lock-probe.test.ts index 60e77c894..c31c214c1 100644 --- a/gitnexus/test/unit/hook-db-lock-probe.test.ts +++ b/gitnexus/test/unit/hook-db-lock-probe.test.ts @@ -17,7 +17,7 @@ import { createRequire } from 'node:module'; import fs from 'fs'; import os from 'os'; import path from 'path'; -import { spawn } from 'child_process'; +import { spawn, spawnSync } from 'child_process'; import { createFakeProcRoot, type FakeProcEntry } from '../utils/hook-test-helpers.js'; const PROBE_PATH = path.resolve(__dirname, '..', '..', 'hooks', 'claude', 'hook-db-lock-probe.cjs'); @@ -27,6 +27,8 @@ type Probe = { linuxProcScanFindGitNexusServer?: (dbPathAbs: string, myPid: number) => string; getCmdlineMaxBytes?: () => number; resolveLinuxProcBudgetMs?: () => number; + resolveHookBinary?: (tool: 'lsof' | 'ps') => string; + hasMissingHookBinaryOverride?: (tool: 'lsof' | 'ps') => boolean; }; const probe = createRequire(import.meta.url)(PROBE_PATH) as Probe; @@ -44,6 +46,8 @@ const ENV_KEYS = [ 'GITNEXUS_HOOK_PROC_ROOT', 'GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS', 'GITNEXUS_HOOK_PROC_CMDLINE_MAX', + 'GITNEXUS_HOOK_LSOF_PATH', + 'GITNEXUS_HOOK_PS_PATH', ] as const; const savedEnv: Record = {}; function setEnv(overrides: Record) { @@ -94,6 +98,49 @@ function runScan( const GITNEXUS_MCP_ARGV = (script: string) => ['node', script, 'mcp']; +// ── Hook binary override: check and lookup agree on trimming (#2543 review) ── +// +// unixLsofPsFindGitNexusServer calls hasMissingHookBinaryOverride (trims) and +// then resolveHookBinary. If the lookup did NOT trim, a padded-but-valid +// override such as " /tmp/lsof " passed the missing-check yet fell through to +// the built-in/PATH binary. Both helpers must see the same trimmed path. +describe('hook binary override trimming (white-box, #2543 review)', () => { + const resolveBin = probe.resolveHookBinary as (tool: 'lsof' | 'ps') => string; + const missing = probe.hasMissingHookBinaryOverride as (tool: 'lsof' | 'ps') => boolean; + + function makeFakeBinary(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-hookbin-')); + cleanups.push(() => fs.rmSync(dir, { recursive: true, force: true })); + const bin = path.join(dir, 'fake-tool'); + fs.writeFileSync(bin, ''); + return bin; + } + + it.each(['lsof', 'ps'] as const)( + '%s: whitespace-padded override to an existing file resolves to the trimmed path', + (tool) => { + const bin = makeFakeBinary(); + const envKey = tool === 'lsof' ? 'GITNEXUS_HOOK_LSOF_PATH' : 'GITNEXUS_HOOK_PS_PATH'; + setEnv({ [envKey]: ` ${bin}\t ` }); + expect(missing(tool)).toBe(false); + expect(resolveBin(tool)).toBe(bin); + }, + ); + + it.each(['', ' '])('empty/whitespace override %j is ignored by both helpers', (value) => { + setEnv({ GITNEXUS_HOOK_LSOF_PATH: value }); + expect(missing('lsof')).toBe(false); + expect(resolveBin('lsof').trim()).not.toBe(''); + }); + + it('missing-path override is reported missing and never returned by the lookup', () => { + const absent = path.join(os.tmpdir(), 'gitnexus-hookbin-does-not-exist', 'lsof'); + setEnv({ GITNEXUS_HOOK_LSOF_PATH: ` ${absent} ` }); + expect(missing('lsof')).toBe(true); + expect(resolveBin('lsof')).not.toBe(absent); + }); +}); + // ── Numeric env parsing (white-box, #2183 review) ────────────────────── // // getCmdlineMaxBytes / resolveLinuxProcBudgetMs switched from parseInt(.,10) to @@ -104,6 +151,38 @@ const GITNEXUS_MCP_ARGV = (script: string) => ['node', script, 'mcp']; // GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS="" resolve to Number("")===0 => budget 0 => // immediate fail-CLOSED timeout (augment permanently skipped). The added // `&& String(raw).trim()` guard keeps ''/whitespace on the 1200 default. +// GITNEXUS_HOOK_TIMEOUT_PATH relative override (#2543 review): the adapters +// spawn the returned wrapper with the tool request's `cwd`, so the probe must +// hand back an ABSOLUTE path resolved against the directory its existsSync +// check and self-test ran in. The resolution is memoized per module instance, +// so each case runs in a fresh `node` child whose cwd holds a self-testing +// guard script (`-k cmd…` -> exec cmd…). +describe.skipIf(process.platform === 'win32')( + 'GITNEXUS_HOOK_TIMEOUT_PATH relative override resolves to absolute (#2543 review)', + () => { + it.each(['./fake-guard', 'fake-guard'])('override %s -> path.resolve(value)', (value) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-probe-relguard-')); + cleanups.push(() => fs.rmSync(dir, { recursive: true, force: true })); + fs.writeFileSync(path.join(dir, 'fake-guard'), '#!/bin/sh\nshift 3\nexec "$@"\n', { + mode: 0o755, + }); + const script = + `const p=require(${JSON.stringify(PROBE_PATH)});` + + `process.stdout.write(JSON.stringify({cwd:process.cwd(),guard:p.resolveUnixGuardTimeout()}));`; + const child = spawnSync(process.execPath, ['-e', script], { + cwd: dir, + encoding: 'utf-8', + env: { ...process.env, GITNEXUS_HOOK_TIMEOUT_PATH: value }, + timeout: 15000, + }); + expect(child.status).toBe(0); + const out = JSON.parse(child.stdout) as { cwd: string; guard: string | null }; + expect(out.guard).toBe(path.resolve(out.cwd, value)); + expect(path.isAbsolute(out.guard ?? '')).toBe(true); + }); + }, +); + describe('numeric env parsing (white-box, #2183 review)', () => { const budget = probe.resolveLinuxProcBudgetMs as () => number; const cmdlineMax = probe.getCmdlineMaxBytes as () => number; @@ -150,6 +229,36 @@ describe('numeric env parsing (white-box, #2183 review)', () => { setEnv({ GITNEXUS_HOOK_PROC_CMDLINE_MAX: undefined }); expect(cmdlineMax()).toBe(16384); }); + + // #2543 review: an oversized-but-finite override used to reach + // Buffer.allocUnsafe unchanged; the allocation threw, readLinuxCmdline's catch + // returned '' (non-candidate) and a real owner was silently missed. Oversized + // integers now clamp to the 256 KiB ceiling (the escalation's HARD_CEIL). + it.each([ + ['262145', 262144], + [String(2 ** 40), 262144], + [String(Number.MAX_SAFE_INTEGER), 262144], + ['1e300', 262144], + ])('cmdline max: oversized %s clamps to the 256 KiB ceiling', (raw, expected) => { + setEnv({ GITNEXUS_HOOK_PROC_CMDLINE_MAX: raw }); + expect(cmdlineMax()).toBe(expected); + }); + + it.each(['4096', '8000', '16384', '262144'])( + 'cmdline max: in-range integer %s is returned unchanged', + (raw) => { + setEnv({ GITNEXUS_HOOK_PROC_CMDLINE_MAX: raw }); + expect(cmdlineMax()).toBe(Number(raw)); + }, + ); + + it.each(['Infinity', '-Infinity', 'NaN', '8192.5', '8abc', '4095', '-1'])( + 'cmdline max: non-integer / garbage / below-floor %s falls back to the 16384 default', + (raw) => { + setEnv({ GITNEXUS_HOOK_PROC_CMDLINE_MAX: raw }); + expect(cmdlineMax()).toBe(16384); + }, + ); }); describe.skipIf(!isLinux)('Linux cmdline-first DB-owner scan (#2180)', () => { @@ -167,6 +276,23 @@ describe.skipIf(!isLinux)('Linux cmdline-first DB-owner scan (#2180)', () => { expect(owned).toBe(true); }); + it('owned: an oversized GITNEXUS_HOOK_PROC_CMDLINE_MAX (2**40) does not blind the scan (#2543 review)', () => { + // Pre-fix, the 2**40 cap reached Buffer.allocUnsafe, threw, and the catch + // mapped it to '' (non-candidate) => a real owner silently reported false. + const { owned } = runScan( + (lbug) => [ + { + pid: 4243, + comm: 'MainThread', + cmdline: GITNEXUS_MCP_ARGV('/opt/app/node_modules/gitnexus/dist/cli/index.js'), + fdTargets: [lbug], + }, + ], + { GITNEXUS_HOOK_PROC_CMDLINE_MAX: String(2 ** 40) }, + ); + expect(owned).toBe(true); + }); + it('not-owned: a node process that is not a gitnexus server (even holding the lbug) is ignored', () => { const { owned } = runScan((lbug) => [ { @@ -267,7 +393,7 @@ describe.skipIf(!isLinux)('Linux cmdline-first DB-owner scan (#2180)', () => { // // The cmdline shape here is deliberate (Codex): the `gitnexus` token sits in // the SECOND argv (a SHORT node_modules/gitnexus path, well inside the first - // 4 KB chunk) so `if (!hasGitNexus) break` does NOT abort the read; a ~9 KB + // 4 KB chunk); a ~9 KB // pad argv then pushes the trailing `mcp` mode token PAST 4096, so the first // 4 KB chunk has gitnexus-but-no-mode and the loop MUST escalate to a second // read to find `mcp`. Setting GITNEXUS_HOOK_PROC_CMDLINE_MAX=4096 makes the @@ -325,6 +451,62 @@ describe.skipIf(!isLinux)('Linux cmdline-first DB-owner scan (#2180)', () => { expect(owned).toBe(false); }); + // ── D3c: a partial read stops early only when DECIDED (#2543 review) ── + // + // The escalation loop used to stop as soon as the chunk held a mode word, or + // as soon as it lacked the gitnexus token. Neither decides ownership: a Node + // preload flag can put `mcp` in the first chunk while the gitnexus CLI path + // lies past it, and a long interpreter prefix can push BOTH tokens past it. + // Either way the old loop returned an incomplete cmdline, Phase 1 rejected it, + // and Phase 2 never compared the holder's fd (a fail-OPEN owner miss, #1492). + // Now only "both tokens present", EOF, the ceiling, or the budget stop it. + + it('owned: a mode word in the first chunk (`--require mcp`) with the gitnexus path past it', () => { + const { owned } = runScan( + (lbug) => [ + { + pid: 10050, + comm: 'MainThread', + // node | --require mcp | 9KB pad | gitnexus path (>4096) | serve + cmdline: ['node', '--require', 'mcp', PAD_PAST_4K, GITNEXUS_SHORT, 'serve'], + fdTargets: [lbug], + }, + ], + { GITNEXUS_HOOK_PROC_CMDLINE_MAX: '4096' }, + ); + expect(owned).toBe(true); + }); + + it('owned: neither token in the first chunk, both past it (long interpreter prefix)', () => { + const { owned } = runScan( + (lbug) => [ + { + pid: 10051, + comm: 'MainThread', + cmdline: ['node', PAD_PAST_4K, GITNEXUS_SHORT, 'mcp'], + fdTargets: [lbug], + }, + ], + { GITNEXUS_HOOK_PROC_CMDLINE_MAX: '4096' }, + ); + expect(owned).toBe(true); + }); + + it('not-owned: a mode word early but no gitnexus token anywhere is read to EOF and rejected', () => { + const { owned } = runScan( + (lbug) => [ + { + pid: 10052, + comm: 'node', + cmdline: ['node', '--require', 'mcp', PAD_PAST_4K, '/app/other-server.js', 'serve'], + fdTargets: [lbug], + }, + ], + { GITNEXUS_HOOK_PROC_CMDLINE_MAX: '4096' }, + ); + expect(owned).toBe(false); + }); + it('does not over-read: a giant non-gitnexus cmdline is bounded and yields not-owned', () => { const giant = 'x'.repeat(500000); // 500 KB single arg, no gitnexus token const { owned } = runScan((lbug) => [ @@ -470,7 +652,8 @@ describe.skipIf(!isLinux)('Linux cmdline-first DB-owner scan (#2180)', () => { // not lock. F1 splits the failure shapes: // - EACCES / EPERM -> 'timeout' (unverifiable; fail-closed HONESTLY) // - EIO / ESTALE -> 'timeout' (transient I/O; fail-closed) - // - ENOTDIR / other -> continue (not a real fd dir; treat as non-owner) + // - ENOTDIR -> continue (not a real fd dir; treat as non-owner) + // - any other errno -> 'timeout' (EMFILE/ENFILE/ENOMEM/…: inconclusive) // The dispatcher collapses owned+timeout to boolean true, so these assert the // exported tri-state verdict directly — a boolean check could not tell the F1 // fix from the old bug. @@ -527,6 +710,12 @@ describe.skipIf(!isLinux)('Linux cmdline-first DB-owner scan (#2180)', () => { { code: 'EIO', expected: 'timeout' }, { code: 'ESTALE', expected: 'timeout' }, { code: 'ENOTDIR', expected: 'not-owned' }, + // Resource/interruption failures say nothing about ownership of an + // already-identified server candidate: fail closed, never not-owned. + { code: 'EMFILE', expected: 'timeout' }, + { code: 'ENFILE', expected: 'timeout' }, + { code: 'ENOMEM', expected: 'timeout' }, + { code: 'EINTR', expected: 'timeout' }, ] as const) { it(`candidate fd readdir ${code} → verdict ${expected} (uid-agnostic spy)`, () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-probe-fderr-')); diff --git a/gitnexus/test/unit/hooks.test.ts b/gitnexus/test/unit/hooks.test.ts index 5d6c74a11..8a7098db5 100644 --- a/gitnexus/test/unit/hooks.test.ts +++ b/gitnexus/test/unit/hooks.test.ts @@ -17,7 +17,7 @@ * Since the hooks are CJS scripts that call main() on load, we test them * by spawning them as child processes with controlled stdin JSON. */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { spawnSync } from 'child_process'; import { createHash } from 'node:crypto'; import { createRequire } from 'node:module'; @@ -800,6 +800,272 @@ describe('PreToolUse concurrency guard', () => { } }); +// ─── Unit: stale-slot eviction never deletes or moves a live slot ── +// +// Eviction deletes a slot only while its per-slot `.evicting` marker still +// carries the evictor's own token, and only if the slot is still the exact +// file judged stale. Markers are removed only after the same kind of check. +// Races are driven deterministically: an fs call is wrapped so a concurrent +// action fires at one exact point of the evictor's sequence. + +type AcquireHookSlot = (gitNexusDir: string) => (() => void) | null; +type WriteFileArgs = Parameters; +type OpenArgs = Parameters; +type LstatArgs = Parameters; + +describe('acquireHookSlot stale-slot eviction', () => { + const DEAD_PID = '2147483640'; + // Another evictor's claim: a token this process never writes. + const FOREIGN_TOKEN = `${process.ppid}:ffffffffffffffff`; + const OWN_TOKEN = new RegExp(`^${process.pid}:[0-9a-f]{16}$`); + + for (const [label, lockPath] of [ + ['CJS', CJS_HOOK_LOCK], + ['Plugin', PLUGIN_HOOK_LOCK], + ] as const) { + const loadAcquire = (): AcquireHookSlot => + (createRequire(import.meta.url)(lockPath) as { acquireHookSlot: AcquireHookSlot }) + .acquireHookSlot; + + const makeLockDir = (): { dir: string; lockDir: string; slot0: string; marker: string } => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-hook-evict-')); + const lockDir = path.join(dir, '.hook-locks'); + fs.mkdirSync(lockDir); + const slot0 = path.join(lockDir, 'slot-0.lock'); + fs.writeFileSync(slot0, DEAD_PID); + return { dir, lockDir, slot0, marker: `${slot0}.evicting` }; + }; + + // Another claimant replaces the marker (a stalled evictor's claim was + // broken and re-taken). Fresh mtime, different size: a new file. + const replaceMarker = (marker: string) => () => { + fs.rmSync(marker, { force: true }); + fs.writeFileSync(marker, FOREIGN_TOKEN, { flag: 'wx' }); + }; + + // Runs `action` once, just before the real write to `target`. + const onWriteTo = (target: string, action: () => void) => { + const realWrite = fs.writeFileSync; + const pending = new Map([[target, action]]); + return vi.spyOn(fs, 'writeFileSync').mockImplementation((...args: WriteFileArgs) => { + const key = String(args[0]); + const fire = pending.get(key); + pending.delete(key); + fire?.(); + realWrite(...args); + }); + }; + + // Runs `action` once, just after the real write to `target`. + const afterWriteTo = (target: string, action: () => void) => { + const realWrite = fs.writeFileSync; + const pending = new Map([[target, action]]); + return vi.spyOn(fs, 'writeFileSync').mockImplementation((...args: WriteFileArgs) => { + realWrite(...args); + const key = String(args[0]); + const fire = pending.get(key); + pending.delete(key); + fire?.(); + }); + }; + + // Runs `action` once, just before the second real open of `target` — i.e. + // between a marker snapshot (open + fstat + read) and its re-check. + const beforeSecondOpenOf = (target: string, action: () => void) => { + const realOpen = fs.openSync; + let targetOpens = 0; + return vi.spyOn(fs, 'openSync').mockImplementation(((...args: OpenArgs) => { + const isTarget = String(args[0]) === target; + targetOpens += Number(isTarget); + [action].filter(() => isTarget && targetOpens === 2).forEach((fire) => fire()); + return realOpen(...args); + }) as typeof fs.openSync); + }; + + // Runs `action` once, just before the first real lstat of `target`. + const beforeFirstLstatOf = (target: string, action: () => void) => { + const realLstat = fs.lstatSync; + const pending = new Map([[target, action]]); + return vi.spyOn(fs, 'lstatSync').mockImplementation(((...args: LstatArgs) => { + const key = String(args[0]); + const fire = pending.get(key); + pending.delete(key); + fire?.(); + return realLstat(...args); + }) as typeof fs.lstatSync); + }; + + it(`${label}: release unregisters its exit listener`, () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-hook-exit-')); + const before = process.listenerCount('exit'); + try { + Array.from({ length: 12 }).forEach(() => loadAcquire()(dir)?.()); + expect(process.listenerCount('exit')).toBe(before); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it(`${label}: evicts a dead-pid slot without renaming it or leaving a marker`, () => { + const { dir, lockDir, slot0 } = makeLockDir(); + const renameSpy = vi.spyOn(fs, 'renameSync'); + const release = loadAcquire()(dir); + try { + expect(release).not.toBeNull(); + expect(fs.readFileSync(slot0, 'utf-8')).toBe(String(process.pid)); + expect(renameSpy).not.toHaveBeenCalled(); + expect(fs.readdirSync(lockDir)).toEqual(['slot-0.lock']); + release?.(); + expect(fs.readdirSync(lockDir)).toEqual([]); + } finally { + renameSpy.mockRestore(); + release?.(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it(`${label}: does not delete a slot recreated between inspection and eviction`, () => { + const { dir, lockDir, slot0, marker } = makeLockDir(); + // A live owner (our parent) whose fresh lock replaces the stale one. + // Different byte length from DEAD_PID so identity cannot collide even + // if the filesystem hands the new file the freed inode number. + const freshOwner = String(process.ppid); + const renameSpy = vi.spyOn(fs, 'renameSync'); + const writeSpy = onWriteTo(marker, () => { + fs.unlinkSync(slot0); + fs.writeFileSync(slot0, freshOwner, { flag: 'wx' }); + }); + const release = loadAcquire()(dir); + try { + expect(writeSpy).toHaveBeenCalledWith(marker, expect.stringMatching(OWN_TOKEN), { + flag: 'wx', + }); + // The fresh lock survived with its owner intact… + expect(fs.readFileSync(slot0, 'utf-8')).toBe(freshOwner); + // …so this contender respected it and took the next slot instead. + expect(release).not.toBeNull(); + expect(fs.readFileSync(path.join(lockDir, 'slot-1.lock'), 'utf-8')).toBe( + String(process.pid), + ); + expect(renameSpy).not.toHaveBeenCalled(); + expect(fs.readdirSync(lockDir).sort()).toEqual(['slot-0.lock', 'slot-1.lock']); + } finally { + writeSpy.mockRestore(); + renameSpy.mockRestore(); + release?.(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it(`${label}: an evictor whose marker was replaced does not delete the slot`, () => { + const { dir, lockDir, slot0, marker } = makeLockDir(); + // Right after this evictor claims the marker, it loses the claim (as if + // stalled past the orphan threshold and broken by another evictor). + const writeSpy = afterWriteTo(marker, replaceMarker(marker)); + const release = loadAcquire()(dir); + try { + // The slot belongs to the new marker holder, not to this evictor… + expect(fs.readFileSync(slot0, 'utf-8')).toBe(DEAD_PID); + // …whose claim this evictor did not remove on the way out… + expect(fs.readFileSync(marker, 'utf-8')).toBe(FOREIGN_TOKEN); + // …and this contender moved on to the next slot. + expect(release).not.toBeNull(); + expect(fs.readFileSync(path.join(lockDir, 'slot-1.lock'), 'utf-8')).toBe( + String(process.pid), + ); + } finally { + writeSpy.mockRestore(); + release?.(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it(`${label}: leaves a marker another evictor claimed during eviction`, () => { + const { dir, lockDir, slot0, marker } = makeLockDir(); + // After this evictor's token check, just before its slot identity + // check, its marker passes to another claimant. + const lstatSpy = beforeFirstLstatOf(slot0, replaceMarker(marker)); + const release = loadAcquire()(dir); + try { + expect(release).not.toBeNull(); + expect(fs.readFileSync(slot0, 'utf-8')).toBe(String(process.pid)); + // The finally block saw a foreign token and left the new claim alone. + expect(fs.readFileSync(marker, 'utf-8')).toBe(FOREIGN_TOKEN); + expect(fs.readdirSync(lockDir).sort()).toEqual(['slot-0.lock', 'slot-0.lock.evicting']); + } finally { + lstatSpy.mockRestore(); + release?.(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it(`${label}: an evictor holding the marker blocks a second evictor`, () => { + const { dir, lockDir, slot0, marker } = makeLockDir(); + // A concurrent evictor's fresh claim on slot-0. + fs.writeFileSync(marker, FOREIGN_TOKEN, { flag: 'wx' }); + const release = loadAcquire()(dir); + try { + // The stale slot is left to the marker holder, not deleted twice… + expect(fs.readFileSync(slot0, 'utf-8')).toBe(DEAD_PID); + expect(fs.readFileSync(marker, 'utf-8')).toBe(FOREIGN_TOKEN); + // …and this contender moved on to the next slot. + expect(release).not.toBeNull(); + expect(fs.readFileSync(path.join(lockDir, 'slot-1.lock'), 'utf-8')).toBe( + String(process.pid), + ); + expect(fs.readdirSync(lockDir).sort()).toEqual([ + 'slot-0.lock', + 'slot-0.lock.evicting', + 'slot-1.lock', + ]); + } finally { + release?.(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it(`${label}: clears an orphaned stale marker and evicts`, () => { + const { dir, lockDir, slot0, marker } = makeLockDir(); + // A crashed evictor's marker, far older than any live critical section. + fs.writeFileSync(marker, DEAD_PID, { flag: 'wx' }); + fs.utimesSync(marker, 1000, 1000); + const release = loadAcquire()(dir); + try { + expect(release).not.toBeNull(); + expect(fs.readFileSync(slot0, 'utf-8')).toBe(String(process.pid)); + expect(fs.readdirSync(lockDir)).toEqual(['slot-0.lock']); + } finally { + release?.(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it(`${label}: does not break a fresh marker that replaced a stale one`, () => { + const { dir, lockDir, slot0, marker } = makeLockDir(); + fs.writeFileSync(marker, DEAD_PID, { flag: 'wx' }); + fs.utimesSync(marker, 1000, 1000); + // Once the old marker has been judged stale, its holder finishes and a + // new evictor claims a fresh one before the orphan unlink. + const readSpy = beforeSecondOpenOf(marker, replaceMarker(marker)); + const release = loadAcquire()(dir); + try { + // The fresh claim stands, and the slot is left to its holder… + expect(fs.readFileSync(marker, 'utf-8')).toBe(FOREIGN_TOKEN); + expect(fs.readFileSync(slot0, 'utf-8')).toBe(DEAD_PID); + // …while this contender took the next slot. + expect(release).not.toBeNull(); + expect(fs.readFileSync(path.join(lockDir, 'slot-1.lock'), 'utf-8')).toBe( + String(process.pid), + ); + } finally { + readSpy.mockRestore(); + release?.(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + } +}); + // ─── Integration: concurrency guard skips when slots are full ────── // The burst tests spawn real child processes; under CI load a child can exit @@ -3795,6 +4061,156 @@ describe('Hook registry resolver compatibility', () => { } }); + // The CLI rejects a set-but-invalid override (storage-resolver.ts + // validateConfiguredStoragePath / storagePathFromRoot), so the hook must + // resolve no repo instead of augmenting from the registry row's storagePath. + // (No NUL row: Node truncates process.env values at NUL, so it cannot be set.) + it.each([ + ['GITNEXUS_STORAGE_PATH', 'relative', 'relative/indexes'], + ['GITNEXUS_STORAGE_PATH', 'filesystem-root', path.parse(os.tmpdir()).root], + ['GITNEXUS_STORAGE_ROOT', 'relative', 'relative/indexes'], + // '' is configured-but-invalid in the CLI (value !== undefined), not unset. + ['GITNEXUS_STORAGE_PATH', 'empty', ''], + ['GITNEXUS_STORAGE_ROOT', 'empty', ''], + ] as const)( + 'resolves no repo when %s is set to a %s value, ignoring the registry storagePath', + (envName, _kind, value) => { + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-hook-home-')); + const repoDir = path.join(homeDir, 'repo'); + const registeredStorage = path.join(homeDir, 'indexes', 'registered'); + const previousPath = process.env.GITNEXUS_STORAGE_PATH; + const previousRoot = process.env.GITNEXUS_STORAGE_ROOT; + try { + fs.mkdirSync(path.join(repoDir, '.gitnexus'), { recursive: true }); + fs.mkdirSync(registeredStorage, { recursive: true }); + initRepoWithCommit(repoDir); + fs.writeFileSync( + path.join(registeredStorage, 'gitnexus.json'), + JSON.stringify({ + repoPath: repoDir, + storagePath: registeredStorage, + lastCommit: 'registered', + stats: {}, + }), + ); + fs.writeFileSync( + path.join(repoDir, '.gitnexus', 'gitnexus.json'), + JSON.stringify({ + repoPath: repoDir, + storagePath: path.join(repoDir, '.gitnexus'), + lastCommit: 'leftover', + stats: {}, + }), + ); + writeHookRegistry(homeDir, [ + { name: 'repo', path: repoDir, storagePath: registeredStorage }, + ]); + delete process.env.GITNEXUS_STORAGE_PATH; + delete process.env.GITNEXUS_STORAGE_ROOT; + process.env[envName] = value; + + withRegistryHome(homeDir, () => { + expect(loadRegistryQuery().resolveHookRepo(repoDir)).toBeNull(); + }); + } finally { + if (previousPath === undefined) delete process.env.GITNEXUS_STORAGE_PATH; + else process.env.GITNEXUS_STORAGE_PATH = previousPath; + if (previousRoot === undefined) delete process.env.GITNEXUS_STORAGE_ROOT; + else process.env.GITNEXUS_STORAGE_ROOT = previousRoot; + fs.rmSync(homeDir, { recursive: true, force: true }); + } + }, + ); + + it('uses the registry storagePath when GITNEXUS_STORAGE_PATH and GITNEXUS_STORAGE_ROOT are unset', () => { + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-hook-home-')); + const repoDir = path.join(homeDir, 'repo'); + const registeredStorage = path.join(homeDir, 'indexes', 'registered'); + const previousPath = process.env.GITNEXUS_STORAGE_PATH; + const previousRoot = process.env.GITNEXUS_STORAGE_ROOT; + try { + fs.mkdirSync(path.join(repoDir, '.gitnexus'), { recursive: true }); + fs.mkdirSync(registeredStorage, { recursive: true }); + initRepoWithCommit(repoDir); + fs.writeFileSync( + path.join(registeredStorage, 'gitnexus.json'), + JSON.stringify({ + repoPath: repoDir, + storagePath: registeredStorage, + lastCommit: 'registered', + stats: {}, + }), + ); + fs.writeFileSync( + path.join(repoDir, '.gitnexus', 'gitnexus.json'), + JSON.stringify({ + repoPath: repoDir, + storagePath: path.join(repoDir, '.gitnexus'), + lastCommit: 'leftover', + stats: {}, + }), + ); + writeHookRegistry(homeDir, [{ name: 'repo', path: repoDir, storagePath: registeredStorage }]); + delete process.env.GITNEXUS_STORAGE_PATH; + delete process.env.GITNEXUS_STORAGE_ROOT; + + withRegistryHome(homeDir, () => { + expect(loadRegistryQuery().resolveHookRepo(repoDir)).toMatchObject({ + path: repoDir, + storagePath: path.resolve(registeredStorage), + metadata: expect.objectContaining({ lastCommit: 'registered' }), + }); + }); + } finally { + if (previousPath === undefined) delete process.env.GITNEXUS_STORAGE_PATH; + else process.env.GITNEXUS_STORAGE_PATH = previousPath; + if (previousRoot === undefined) delete process.env.GITNEXUS_STORAGE_ROOT; + else process.env.GITNEXUS_STORAGE_ROOT = previousRoot; + fs.rmSync(homeDir, { recursive: true, force: true }); + } + }); + + it('resolveHookRepo prefers a valid absolute GITNEXUS_STORAGE_PATH over the registry storagePath', () => { + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-hook-home-')); + const repoDir = path.join(homeDir, 'repo'); + const registeredStorage = path.join(homeDir, 'indexes', 'registered'); + const overrideStorage = path.join(homeDir, 'indexes', 'override'); + const previousPath = process.env.GITNEXUS_STORAGE_PATH; + const previousRoot = process.env.GITNEXUS_STORAGE_ROOT; + try { + fs.mkdirSync(repoDir, { recursive: true }); + fs.mkdirSync(registeredStorage, { recursive: true }); + fs.mkdirSync(overrideStorage, { recursive: true }); + initRepoWithCommit(repoDir); + for (const [storage, lastCommit] of [ + [registeredStorage, 'registered'], + [overrideStorage, 'override'], + ] as const) { + fs.writeFileSync( + path.join(storage, 'gitnexus.json'), + JSON.stringify({ repoPath: repoDir, storagePath: storage, lastCommit, stats: {} }), + ); + } + writeHookRegistry(homeDir, [{ name: 'repo', path: repoDir, storagePath: registeredStorage }]); + delete process.env.GITNEXUS_STORAGE_ROOT; + process.env.GITNEXUS_STORAGE_PATH = overrideStorage; + + withRegistryHome(homeDir, () => { + expect(loadRegistryQuery().resolveHookRepo(repoDir)).toMatchObject({ + path: repoDir, + storagePath: path.resolve(overrideStorage), + metadata: expect.objectContaining({ lastCommit: 'override' }), + }); + }); + } finally { + if (previousPath === undefined) delete process.env.GITNEXUS_STORAGE_PATH; + else process.env.GITNEXUS_STORAGE_PATH = previousPath; + if (previousRoot === undefined) delete process.env.GITNEXUS_STORAGE_ROOT; + else process.env.GITNEXUS_STORAGE_ROOT = previousRoot; + fs.rmSync(homeDir, { recursive: true, force: true }); + } + }); + it('prefers a registered external slot over leftover local .gitnexus', () => { const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-hook-home-')); const repoDir = path.join(homeDir, 'repo'); diff --git a/gitnexus/test/unit/setup-selection.test.ts b/gitnexus/test/unit/setup-selection.test.ts index 7978ec2dc..a57689550 100644 --- a/gitnexus/test/unit/setup-selection.test.ts +++ b/gitnexus/test/unit/setup-selection.test.ts @@ -101,7 +101,7 @@ describe('setupCommand coding-agent selection', () => { expect(process.exitCode).toBe(1); expect(stderr).toHaveBeenCalledWith( expect.stringContaining( - 'Valid values: cursor, claude, antigravity, opencode, codebuddy, qoder, codex', + 'Valid values: cursor, claude, antigravity, opencode, codebuddy, qoder, codex, droid', ), ); await expect( diff --git a/gitnexus/test/unit/setup.test.ts b/gitnexus/test/unit/setup.test.ts index cff1b300a..942211ae3 100644 --- a/gitnexus/test/unit/setup.test.ts +++ b/gitnexus/test/unit/setup.test.ts @@ -720,6 +720,82 @@ describe('setupQoder', () => { }); }); +describe('setupDroid (Factory)', () => { + let tempHome: string; + let originalHome: string | undefined; + let originalUserProfile: string | undefined; + + const configPath = () => path.join(tempHome, '.factory', 'mcp.json'); + + beforeEach(async () => { + vi.resetModules(); + vi.clearAllMocks(); + + originalHome = process.env.HOME; + originalUserProfile = process.env.USERPROFILE; + tempHome = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-droid-setup-')); + process.env.HOME = tempHome; + process.env.USERPROFILE = tempHome; + + // Only create ~/.factory so other editors skip and don't pollute assertions. + await fs.mkdir(path.join(tempHome, '.factory'), { recursive: true }); + + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + // Assigning undefined would set the string "undefined"; delete instead. + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + await fs.rm(tempHome, { recursive: true, force: true }); + }); + + it('writes the MCP entry to ~/.factory/mcp.json under mcpServers', async () => { + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const config = JSON.parse(await fs.readFile(configPath(), 'utf-8')); + expect(config.mcpServers.gitnexus).toBeDefined(); + }); + + it('preserves existing servers in ~/.factory/mcp.json', async () => { + await fs.writeFile( + configPath(), + JSON.stringify({ mcpServers: { other: { command: 'foo' } } }), + 'utf-8', + ); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const config = JSON.parse(await fs.readFile(configPath(), 'utf-8')); + expect(config.mcpServers.other).toEqual({ command: 'foo' }); + expect(config.mcpServers.gitnexus).toBeDefined(); + }); + + it('skips when ~/.factory directory does not exist', async () => { + await fs.rm(path.join(tempHome, '.factory'), { recursive: true, force: true }); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + await expect(fs.access(configPath())).rejects.toThrow(); + }); + + it('leaves a corrupt ~/.factory/mcp.json untouched', async () => { + const corrupt = '{ this is not valid json !!!'; + await fs.writeFile(configPath(), corrupt, 'utf-8'); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + expect(await fs.readFile(configPath(), 'utf-8')).toBe(corrupt); + }); +}); + describe('Codex hooks (installClaudeSchemaHooks)', () => { let tempHome: string; let originalHome: string | undefined; diff --git a/gitnexus/test/unit/sync-plugin-manifests.test.ts b/gitnexus/test/unit/sync-plugin-manifests.test.ts index e932bad4a..3c3d598f1 100644 --- a/gitnexus/test/unit/sync-plugin-manifests.test.ts +++ b/gitnexus/test/unit/sync-plugin-manifests.test.ts @@ -9,8 +9,12 @@ const SURFACES = [ '.claude-plugin/marketplace.json', 'gitnexus-claude-plugin/.codex-plugin/plugin.json', '.agents/plugins/marketplace.json', + 'gitnexus-factory-plugin/.factory-plugin/plugin.json', + '.factory-plugin/marketplace.json', ] as const; +const FACTORY_MCP = 'gitnexus-factory-plugin/mcp.json'; + const MCP_SKILL_DIRS = [ 'gitnexus-plan', 'gitnexus-work', @@ -24,12 +28,14 @@ const MCP_SKILL_DIRS = [ 'gitnexus-refactoring', ] as const; -const TOTAL_SURFACES = SURFACES.length + MCP_SKILL_DIRS.length; - function mcpPath(dir: string): string { return `gitnexus-claude-plugin/skills/${dir}/mcp.json`; } +const EXECUTABLE_MCP_FILES = [...MCP_SKILL_DIRS.map(mcpPath), FACTORY_MCP]; + +const TOTAL_SURFACES = SURFACES.length + EXECUTABLE_MCP_FILES.length; + const tempRoots: string[] = []; afterEach(() => { @@ -56,8 +62,13 @@ function makeRoot(packageVersion: string, manifestVersion: string): string { name: 'gitnexus-marketplace', plugins: [{ name: 'gitnexus', version: manifestVersion, category: 'Developer Tools' }], }); - for (const dir of MCP_SKILL_DIRS) { - writeJson(root, mcpPath(dir), { + writeJson(root, SURFACES[4], { name: 'gitnexus', version: manifestVersion }); + writeJson(root, SURFACES[5], { + name: 'gitnexus-marketplace', + plugins: [{ name: 'gitnexus', version: manifestVersion, source: './gitnexus-factory-plugin' }], + }); + for (const dir of EXECUTABLE_MCP_FILES) { + writeJson(root, dir, { mcpServers: { gitnexus: { command: 'npx', args: ['-y', `gitnexus@${manifestVersion}`, 'mcp'] }, }, @@ -89,19 +100,19 @@ describe('syncPluginManifests (#2445)', () => { expect(result.version).toBe('1.6.10-rc.29'); expect(result.synced).toHaveLength(TOTAL_SURFACES); expect(result.stale.map(({ from }) => from)).toEqual(Array(TOTAL_SURFACES).fill('1.6.9')); - expect(readVersions(root)).toEqual(Array(4).fill('1.6.10-rc.29')); + expect(readVersions(root)).toEqual(Array(SURFACES.length).fill('1.6.10-rc.29')); }); - it('pins the gitnexus@ launch arg in every plugin skill mcp.json', () => { + it('pins the gitnexus@ launch arg in every executable mcp.json', () => { const root = makeRoot('1.6.10-rc.29', '1.6.9'); syncPluginManifests(root); - for (const dir of MCP_SKILL_DIRS) { - const mcp = JSON.parse(readFileSync(path.join(root, mcpPath(dir)), 'utf8')) as { + for (const file of EXECUTABLE_MCP_FILES) { + const mcp = JSON.parse(readFileSync(path.join(root, file), 'utf8')) as { mcpServers: { gitnexus: { args: string[] } }; }; - expect(mcp.mcpServers.gitnexus.args).toEqual(['-y', 'gitnexus@1.6.10-rc.29', 'mcp']); + expect(mcp.mcpServers.gitnexus.args, file).toEqual(['-y', 'gitnexus@1.6.10-rc.29', 'mcp']); } }); @@ -131,7 +142,7 @@ describe('syncPluginManifests (#2445)', () => { expect(result.stale).toHaveLength(TOTAL_SURFACES); expect(result.synced).toHaveLength(0); - expect(readVersions(root)).toEqual(Array(4).fill('1.6.9')); + expect(readVersions(root)).toEqual(Array(SURFACES.length).fill('1.6.9')); }); it('changes only the version text and preserves the surrounding formatting', () => { @@ -200,4 +211,22 @@ describe('syncPluginManifests (#2445)', () => { expect(pkg.default.scripts.version).toBe('node scripts/sync-plugin-manifests.mjs'); }); + + it('stages the synced manifest surfaces in the detached rc release commit', () => { + const workflow = readFileSync( + path.resolve(__dirname, '..', '..', '..', '.github', 'workflows', 'publish.yml'), + 'utf8', + ); + const start = workflow.indexOf('# The synced manifest surfaces (#2445)'); + const end = workflow.indexOf('git commit -m "release: ${VTAG}"', start); + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + const releaseCommit = workflow.slice(start, end); + // The step runs in gitnexus/, so repo-root surfaces are staged as `../`. + const staged = [...releaseCommit.matchAll(/\.\.\/(\S+\.json)/g)].map(([, file]) => file); + + // `--check` reads the working tree, so a surface synced but not staged + // passes CI while the v tag's tree keeps the previous version. + expect(staged).toEqual(expect.arrayContaining([...SURFACES, ...EXECUTABLE_MCP_FILES])); + }); });