This commit is contained in:
luyua9 2026-10-20 07:58:18 +00:00 • committed by GitHub
commit 6fd1a785f5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 474 additions and 3 deletions

View file

@ -339,6 +339,179 @@ function resolveCliPath() {
}
}
// The Cursor host enforces hooks.json's postToolUse `timeout` (seconds)
// against the WHOLE hook process, so the npx fallback can never get more
// than that window however long a cold `npx -y gitnexus` install (package
// download + unpack) actually takes. hooks.json ships 60s for exactly that
// download; this hook then derives the npx budget from what is LEFT of that
// window rather than from the caller's inner-CLI timeout, which was sized
// for an already-installed local CLI and would otherwise pin every cold
// install at ~7s + 5s — the very cost the 60s manifest was raised for.
//
// Three bounds, all applied by resolveNpxTimeoutMs() below:
// HEADROOM — reserved for node startup, the hook-slot release, and the
// final stdout write the host still has to read;
// MIN — below this, the remaining window is too short for a result
// the host could ever observe (the guard's SIGKILL grace, plus
// spawnSync teardown, plus the final write), so the npx
// fallback is SKIPPED outright instead of starting work that
// is guaranteed to be killed mid-flight;
// MAX — upper clamp so one pathologically slow install cannot hold a
// per-repo LadybugDB hook slot for the whole host window and
// starve concurrent edits in the same repo.
const CURSOR_NPX_HEADROOM_MS = 5000;
const CURSOR_NPX_MIN_BUDGET_MS = 5000;
const CURSOR_NPX_MAX_BUDGET_MS = 45000;
const CURSOR_HOOK_START_MS = Date.now();
const HOOK_ENV = process['env'];
function resolveCursorHostBudgetMs() {
try {
// hooks.json ships next to this hook script
const manifest = JSON.parse(fs.readFileSync(path.join(__dirname, 'hooks.json'), 'utf-8'));
const seconds = manifest.hooks.postToolUse[0].timeout;
if (typeof seconds === 'number' && seconds > 0) {
return seconds * 1000;
}
} catch {
/* fall through to the shipped default */
}
return 60000;
}
// Decide the npx fallback's budget from the host window that is actually
// left. Returns null when nothing useful fits — the caller must then skip
// the npx spawn entirely rather than floor the budget at some minimum.
//
// Two failure shapes this replaces, both silent:
// - a `Math.max(1000, …)` floor meant an ALREADY-EXPIRED window still
// handed the guarded arm 1000ms (plus a 2000ms spawnSync allowance), so
// the host killed the hook before the guard could return or the final
// stdout write could land. Starting work that cannot finish is strictly
// worse than not starting it;
// - an inner `timeout + 5000` ceiling capped every fallback at the old
// 7s + 5s budget, so the 60s hooks.json manifest could never actually
// pay for a cold `npx -y gitnexus` download.
//
// Deliberately pure and parameterised so the arithmetic is testable without
// spawning npx, and so it does NOT take the caller's inner-CLI `timeout`:
// that value still sizes the direct-exec arm (where it is correct) and is
// meaningless for a cold package download.
function resolveNpxTimeoutMs(hostBudgetMs, elapsedMs) {
const remaining = hostBudgetMs - CURSOR_NPX_HEADROOM_MS - elapsedMs;
if (remaining < CURSOR_NPX_MIN_BUDGET_MS) return null;
return Math.min(remaining, CURSOR_NPX_MAX_BUDGET_MS);
}
// Sentinel for the module-wide memo below:
// undefined = not resolved yet (resolve lazily, on the first npx fallback)
// string = self-tested coreutils timeout/gtimeout path (use as wrapper)
// null = no usable wrapper (disabled, none found, or self-test failed)
let unixGuardTimeoutCache;
/**
* A candidate is usable only when it RUNS the wrapped command AND
* PROPAGATES its exit status, proven by
* `timeout -k 1 1 /bin/sh -c 'exit 42'` exiting 42.
*
* This rejects two shapes that mere existence cannot:
* - wrappers WITHOUT coreutils' `-k` flag — busybox <1.34, toybox, broken
* symlinks — which exit with a usage error without ever running npx, so
* the npx grandchild is neither wrapped nor reaped;
* - always-exit-0 stubs (/bin/true shapes), which would be adopted and
* "succeed" every wrapped spawn instantly without running it. Status 0
* and empty stderr satisfy main()'s `!child.error && child.status === 0`
* check, so the hook would emit NO additional_context at all — a
* silently dead augmentation rather than a visible failure.
*/
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;
}
}
/**
* Resolve a coreutils/BSD `timeout` binary for the npx arm. The CLI behind
* npx is a grandchild (npx -> gitnexus), so the wrap leads with `-s KILL`:
* a plain SIGTERM would kill only the obedient npx parent and let the
* grandchild keep holding the LadybugDB lock after this hook released its
* slot. Mirrors the Claude adapter's wrapped npx arm; Windows stays unwrapped
* (npx is a .cmd script there).
*
* GITNEXUS_HOOK_TIMEOUT_PATH semantics (shared with the Claude adapter's
* hook-db-lock-probe.cjs, which owns the canonical contract): the sentinel
* `disabled` turns the wrapper off; any other value is only a CANDIDATE — an
* existing 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`, always-exit-0 stub) resolution
* falls through to the built-ins below, tried in order, first self-test pass
* wins. Only when EVERY candidate fails does this fall back to the unwrapped
* status quo. This is strictly stronger than merely checking existence: no
* bad env value of ANY shape can silently disable orphan containment.
*
* Memoized per hook process so the resolution — and its one self-test spawn
* — happens at most once however many times runGitNexusCli falls through to
* npx, and so the chosen wrapper stays stable for the process lifetime. (The
* claude adapter memoizes module-wide so probe and adapter share one
* self-test; this integration ships WITHOUT the probe — see main() — so the
* memo exists purely to bound the self-test spawns.)
*
* Residual gap, same as the claude adapter: the self-test proves `-k` exit
* propagation, not process-GROUP signalling. busybox >=1.34 passes it and is
* fully usable for the direct-exec arm, but only signals its direct child,
* so on busybox the `-s KILL` below may not reap the npx -> gitnexus
* grandchild.
*/
function resolveUnixGuardTimeout() {
if (unixGuardTimeoutCache !== undefined) return unixGuardTimeoutCache;
unixGuardTimeoutCache = null;
if (process.platform === 'win32') return unixGuardTimeoutCache;
const fromEnv = HOOK_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. runGitNexusCli spawns 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',
// Homebrew's coreutils keg installs the GNU binary as `gtimeout`, NOT
// `timeout` — /opt/homebrew/bin/timeout and /usr/local/bin/timeout do
// not exist on any Homebrew install. Listing the un-prefixed names
// (as an earlier revision did) meant the `-s KILL` orphan-containment
// wrap silently never engaged on exactly the platforms where Homebrew is
// how coreutils gets installed.
'/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 runGitNexusCli(cliPath, args, cwd, timeout) {
const isWin = process.platform === 'win32';
if (cliPath) {
@ -350,9 +523,53 @@ function runGitNexusCli(cliPath, args, cwd, timeout) {
windowsHide: true,
});
}
const elapsed = Date.now() - CURSOR_HOOK_START_MS;
const npxTimeout = resolveNpxTimeoutMs(resolveCursorHostBudgetMs(), elapsed);
// Nothing usable is left of the host window, so the npx fallback is SKIPPED
// rather than started. Starting it anyway would only hand the guard a slice
// of an already-expired budget and let the host kill the hook mid-flight —
// which loses the augmentation AND delays the hook-slot release that the
// LadybugDB concurrency guard is holding on our behalf.
//
// Return the same shape spawnSync produces for a child that never ran:
// no `error`, no exit `status`, empty streams. main() already gates on
// `!child.error && child.status === 0`, so this lands on the ordinary
// "no context to emit" path and its `finally` still releases the slot.
if (npxTimeout === null) return { status: null, signal: null, stdout: '', stderr: '' };
const guard = resolveUnixGuardTimeout();
if (guard) {
const wrapped = spawnSync(
guard,
[
'-s',
'KILL',
'-k',
'1',
String(Math.ceil(npxTimeout / 1000) + 1),
'npx',
'-y',
'gitnexus',
...args,
],
{
encoding: 'utf-8',
// npxTimeout already excludes CURSOR_NPX_HEADROOM_MS from the host
// window, so this +2000 allowance for spawnSync's own SIGTERM-then-
// wait teardown still lands inside that reserved headroom.
timeout: npxTimeout + 2000,
cwd,
stdio: ['pipe', 'pipe', 'pipe'],
windowsHide: true,
},
);
if (!wrapped.error || wrapped.error.code !== 'ENOENT') {
return wrapped;
}
// guard vanished between probe and spawn: fall through unwrapped
}
return spawnSync(isWin ? 'npx.cmd' : 'npx', ['-y', 'gitnexus', ...args], {
encoding: 'utf-8',
timeout: timeout + 5000,
timeout: npxTimeout,
cwd,
stdio: ['pipe', 'pipe', 'pipe'],
windowsHide: true,
@ -427,4 +644,13 @@ function main() {
if (require.main === module) main();
module.exports = { parseRgGrepPattern, tokenizeShellWords };
// resolveUnixGuardTimeout / resolveNpxTimeoutMs are exported for the unit
// tests, which exercise the real resolver and budget arithmetic rather than
// grepping this file for substrings. They are pure helpers with no effect on
// the hook's own execution path.
module.exports = {
parseRgGrepPattern,
tokenizeShellWords,
resolveUnixGuardTimeout,
resolveNpxTimeoutMs,
};

View file

@ -5,7 +5,7 @@
{
"matcher": "Shell|Read|Grep",
"command": "node ./hooks/gitnexus-hook.cjs",
"timeout": 10
"timeout": 60
}
]
}

View file

@ -441,6 +441,251 @@ describe('Cursor hook debug logging', () => {
});
});
// ─── Source code regression: npx fallback stays under the host budget ─────
describe('Cursor hook npx fallback host budget', () => {
const source = fs.readFileSync(CURSOR_HOOK, 'utf-8');
const manifest = JSON.parse(
fs.readFileSync(path.join(path.dirname(CURSOR_HOOK), 'hooks.json'), 'utf-8'),
);
it('ships a postToolUse timeout with room for a cold npx install', () => {
// Cold `npx -y gitnexus` has to download + install the package; the
// original 10s budget killed the hook before the child could ever
// finish. The existing manifest test only pins (0, 120).
const seconds = manifest.hooks.postToolUse[0].timeout;
expect(seconds).toBeGreaterThanOrEqual(30);
expect(seconds).toBeLessThan(120);
});
it('sizes the npx timeout from the host budget with headroom', () => {
// Extract runGitNexusCli so the assertions are tied to the actual
// spawn wiring, not just to unrelated substrings elsewhere.
const fnStart = source.indexOf('function runGitNexusCli');
const fnBody = source.slice(fnStart, source.indexOf('\n}\n', fnStart));
// budget comes from hooks.json (seconds → ms) and the elapsed hook time
// is handed to the pure resolver, which owns the headroom arithmetic
expect(fnBody).toContain('resolveNpxTimeoutMs(resolveCursorHostBudgetMs(), elapsed)');
// the computed budget is what reaches spawnSync (not a bare +5000)
expect(fnBody).toContain('timeout: npxTimeout');
expect(fnBody).not.toMatch(/timeout:\s*timeout\s*\+\s*5000/);
// npx grandchild is killed outright so it cannot keep the DB lock
expect(fnBody).toMatch(/['"]-s['"]/);
expect(fnBody).toMatch(/['"]KILL['"]/);
});
it('reads the shipped timeout value, not a detached literal', () => {
// resolveCursorHostBudgetMs must parse hooks.json so changing the
// manifest cannot silently desync from the hook budget.
expect(source).toContain('postToolUse[0].timeout');
expect(source).toContain('return seconds * 1000;');
});
});
// ─── Behavioral: npx fallback budget arithmetic ─────────────────────
// resolveNpxTimeoutMs is the whole decision, pure and exported, so these
// assert the real numbers rather than grepping the hook for substrings.
describe('Cursor hook npx fallback budget', () => {
const hook = require(CURSOR_HOOK) as {
resolveNpxTimeoutMs: (hostBudgetMs: number, elapsedMs: number) => number | null;
};
const manifest = JSON.parse(
fs.readFileSync(path.join(path.dirname(CURSOR_HOOK), 'hooks.json'), 'utf-8'),
);
const hostBudgetMs: number = manifest.hooks.postToolUse[0].timeout * 1000;
const source = fs.readFileSync(CURSOR_HOOK, 'utf-8');
// Same reason as below: the budget comments quote `timeout + 5000` to
// explain what was removed, so strip `//` comments before matching code.
const sourceCode = source
.split('\n')
.map((line) => line.replace(/\/\/.*$/, ''))
.join('\n');
it('is no longer hard-capped at the old inner+5s budget', () => {
// main() calls runGitNexusCli(..., 7000), so the previous
// `Math.min(timeout + 5000, ...)` ceiling pinned every npx fallback at
// 12000ms — exactly the cold `npx -y gitnexus` download the 60s manifest
// was raised to accommodate. The budget must now follow the host window.
const fresh = hook.resolveNpxTimeoutMs(hostBudgetMs, 0);
expect(fresh).not.toBeNull();
expect(fresh as number).toBeGreaterThan(7000 + 5000);
});
it('shrinks as the hook burns its host window', () => {
// Past the MAX clamp the budget tracks the remaining window one-for-one:
// each elapsed millisecond is one less millisecond for npx.
const early = hook.resolveNpxTimeoutMs(hostBudgetMs, 20_000) as number;
const later = hook.resolveNpxTimeoutMs(hostBudgetMs, 30_000) as number;
expect(later).toBeLessThan(early);
expect(early - later).toBe(10_000);
});
it('returns null once no usable budget remains, instead of a 1s floor', () => {
// `Math.max(1000, ...)` used to hand the guard 1000ms of an ALREADY
// EXPIRED window (plus a 2000ms spawnSync allowance), so the host killed
// the hook before the guard could return or the final stdout write could
// land. Exhausted must mean "do not start npx", never "start it anyway".
expect(hook.resolveNpxTimeoutMs(hostBudgetMs, hostBudgetMs)).toBeNull();
expect(hook.resolveNpxTimeoutMs(hostBudgetMs, hostBudgetMs + 5_000)).toBeNull();
// 1ms of window left after CURSOR_NPX_HEADROOM_MS — still unusable
expect(hook.resolveNpxTimeoutMs(hostBudgetMs, hostBudgetMs - 5_000 - 1)).toBeNull();
});
it('still allows a real budget right at the documented floor', () => {
// Floor is CURSOR_NPX_MIN_BUDGET_MS (5000ms of window left after
// CURSOR_NPX_HEADROOM_MS); one millisecond less skips. Pins the boundary
// so the skip cannot drift into rejecting a budget that could actually
// have completed.
const windowLeft = hostBudgetMs - 5_000; // after headroom
expect(hook.resolveNpxTimeoutMs(hostBudgetMs, windowLeft - 5_000)).toBe(5_000);
expect(hook.resolveNpxTimeoutMs(hostBudgetMs, windowLeft - 4_999)).toBeNull();
});
it('clamps to a sane upper bound rather than the whole host window', () => {
// One pathological install must not hold a per-repo hook slot for the
// full 60s window and starve concurrent edits in the same repo.
expect(hook.resolveNpxTimeoutMs(hostBudgetMs, 0)).toBe(45_000);
// ...and the clamp only binds once the remaining window exceeds it
expect(hook.resolveNpxTimeoutMs(hostBudgetMs, 15_000)).toBe(40_000);
});
it('no longer mentions the removed inner+5s ceiling anywhere', () => {
expect(sourceCode).not.toMatch(/timeout\s*\+\s*5000/);
});
it('skips the npx spawn before resolving a guard when the budget is gone', () => {
// Wiring check (the decision itself is covered behaviourally above):
// the null return has to happen BEFORE the guard self-test spawn and
// before any npx spawnSync, otherwise an exhausted window would still
// pay for a self-test it has no budget to use.
const fnStart = source.indexOf('function runGitNexusCli');
const fnBody = source.slice(fnStart, source.indexOf('\n}\n', fnStart));
const skipAt = fnBody.indexOf('if (npxTimeout === null)');
expect(skipAt).toBeGreaterThan(-1);
expect(fnBody.indexOf('resolveUnixGuardTimeout()')).toBeGreaterThan(skipAt);
expect(fnBody.indexOf('npx.cmd')).toBeGreaterThan(skipAt);
});
});
// ─── Behavioral: guard timeout resolution (F1 + F2) ────────────────
// resolveUnixGuardTimeout memoizes per module instance, so each case loads
// a FRESH copy of the hook rather than reusing the top-of-file require.
describe('Cursor hook guard timeout resolution', () => {
const isUnix = process.platform !== 'win32';
const source = fs.readFileSync(CURSOR_HOOK, 'utf-8');
// This hook is deliberately dense with explanatory comments — several of
// them quote the very paths being asserted on (to explain what an earlier
// revision got wrong). Strip `//` comments so these source assertions
// describe CODE, and cannot pass or fail on the prose around it.
function codeOf(fnSignature: string): string {
const start = source.indexOf(fnSignature);
expect(start).toBeGreaterThan(-1);
const body = source.slice(start, source.indexOf('\n}\n', start));
return body
.split('\n')
.map((line) => line.replace(/\/\/.*$/, ''))
.join('\n');
}
function freshHook(): {
resolveUnixGuardTimeout: () => string | null;
} {
delete require.cache[require.resolve(CURSOR_HOOK)];
return require(CURSOR_HOOK) as { resolveUnixGuardTimeout: () => string | null };
}
function withTimeoutPath<T>(value: string | undefined, fn: () => T): T {
const prior = process.env.GITNEXUS_HOOK_TIMEOUT_PATH;
if (value === undefined) delete process.env.GITNEXUS_HOOK_TIMEOUT_PATH;
else process.env.GITNEXUS_HOOK_TIMEOUT_PATH = value;
try {
return fn();
} finally {
if (prior === undefined) delete process.env.GITNEXUS_HOOK_TIMEOUT_PATH;
else process.env.GITNEXUS_HOOK_TIMEOUT_PATH = prior;
}
}
it('looks for Homebrew gtimeout, not a Homebrew timeout', () => {
// Homebrew's coreutils keg installs the GNU binary as `gtimeout`. Listing
// `/opt/homebrew/bin/timeout` (as an earlier revision did) meant the
// `-s KILL` orphan-containment wrap silently never engaged on Apple
// Silicon, because every candidate is an absolute path and so the old
// `which` fallback could never discover `gtimeout` either.
const fnBody = codeOf('function resolveUnixGuardTimeout');
expect(fnBody).toContain('/opt/homebrew/bin/gtimeout');
expect(fnBody).toContain('/usr/local/bin/gtimeout');
expect(fnBody).toContain('/usr/bin/timeout');
expect(fnBody).toContain('/bin/timeout');
expect(fnBody).not.toContain('/opt/homebrew/bin/timeout');
expect(fnBody).not.toContain('/usr/local/bin/timeout');
// The `which` probe is gone: it could never fire for absolute paths and
// would only re-introduce PATH-dependent, unvalidated adoption.
expect(fnBody).not.toContain('which');
});
it.skipIf(!isUnix)('does not adopt an always-exit-0 stub as the guard', () => {
// GITNEXUS_HOOK_TIMEOUT_PATH=/bin/true passes a bare existsSync
// check. The wrapped spawn then "succeeded" instantly without ever
// running npx, and status 0 + empty stderr satisfies main()'s
// `!child.error && child.status === 0` check — so the hook emitted NO
// augmentation at all: a silently dead hook, not a visible failure.
// Hermetic stub (not /bin/true, which macOS 15+ no longer ships).
const stubDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-guard-stub-'));
const stub = path.join(stubDir, 'stub-timeout');
try {
fs.writeFileSync(stub, '#!/bin/sh\nexit 0\n', { mode: 0o755 });
const resolved = withTimeoutPath(stub, () => freshHook().resolveUnixGuardTimeout());
expect(resolved).not.toBe(stub);
} finally {
fs.rmSync(stubDir, { recursive: true, force: true });
}
});
it.skipIf(!isUnix)('resolves a relative override against this process, not the tool cwd', () => {
// runGitNexusCli spawns with the tool request's cwd, so a relative
// override would ENOENT there — or, slashless, silently become a PATH
// lookup. path.resolve makes the adopted path absolute.
const resolved = withTimeoutPath('some/wrapper', () => freshHook().resolveUnixGuardTimeout());
expect(resolved).not.toBe('some/wrapper');
});
it.skipIf(!isUnix)('honours the disabled sentinel', () => {
expect(withTimeoutPath('disabled', () => freshHook().resolveUnixGuardTimeout())).toBeNull();
});
it.skipIf(!isUnix)(
'only ever adopts a guard that runs the command AND propagates its exit status',
() => {
// Independent re-derivation of the contract: whatever the resolver
// returns must be a real coreutils timeout/gtimeout AND must actually
// propagate a non-zero exit. null is also a valid answer (no wrapper
// installed here), but never a silently-dead substitute.
const resolved = withTimeoutPath(undefined, () => freshHook().resolveUnixGuardTimeout());
if (resolved === null) return; // nothing installed to validate
expect(path.basename(resolved)).toMatch(/^g?timeout$/);
const probe = spawnSync(resolved, ['-k', '1', '1', '/bin/sh', '-c', 'exit 42'], {
encoding: 'utf-8',
timeout: 5000,
stdio: ['ignore', 'ignore', 'ignore'],
});
expect(probe.error).toBeUndefined();
expect(probe.status).toBe(42);
},
);
it.skipIf(!isUnix)('self-tests candidates with the -k exit-propagation probe', () => {
// The adoption test itself, pinned so it cannot be weakened back to an
// existence check: it must run a wrapped /bin/sh and observe exit 42.
expect(source).toContain("['-k', '1', '1', '/bin/sh', '-c', 'exit 42']");
expect(source).toContain('!selfTest.error && selfTest.status === 42');
});
});
// ─── Source code regression: concurrency guard (#1486) ─────────────
describe('Cursor hook concurrency guard', () => {