Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions gitnexus/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion gitnexus/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"mnemonist": "^0.40.3",
"onnxruntime-node": "^1.24.0",
"pandemonium": "^2.4.0",
"systeminformation": "^5.31.5",
"tree-sitter": "^0.21.1",
"tree-sitter-c": "0.23.2",
"tree-sitter-c-sharp": "0.23.1",
Expand All @@ -92,14 +93,14 @@
"tree-sitter-swift": "^0.6.0"
},
"devDependencies": {
"gitnexus-shared": "file:../gitnexus-shared",
"@types/cli-progress": "^3.11.6",
"@types/cors": "^2.8.17",
"@types/express": "^4.17.21",
"@types/js-yaml": "^4.0.9",
"@types/node": "^25.6.0",
"@types/uuid": "^10.0.0",
"@vitest/coverage-v8": "^4.0.18",
"gitnexus-shared": "file:../gitnexus-shared",
"tsx": "^4.0.0",
"typescript": "^5.4.5",
"vitest": "^4.0.18"
Expand Down
202 changes: 201 additions & 1 deletion gitnexus/src/cli/analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,25 @@
*/

import path from 'path';
import { execFileSync } from 'child_process';
import { execFileSync, spawnSync } from 'child_process';
import v8 from 'v8';
import cliProgress from 'cli-progress';
import { closeLbug } from '../core/lbug/lbug-adapter.js';
import {
getStoragePaths,
getGlobalRegistryPath,
listRegisteredRepos,
RegistryNameCollisionError,
} from '../storage/repo-manager.js';
import { getGitRoot, hasGitDir } from '../storage/git.js';
import { runFullAnalysis } from '../core/run-analyze.js';
import {
readThresholdsFromEnv,
waitForResourceAvailability,
warnIfThresholdsRisky,
} from './resource-throttle.js';
import fs from 'fs/promises';
import fsSync from 'fs';

const HEAP_MB = 8192;
const HEAP_FLAG = `--max-old-space-size=${HEAP_MB}`;
Expand Down Expand Up @@ -78,15 +85,208 @@ export interface AnalyzeOptions {
* `allowDuplicateName` option end-to-end.
*/
allowDuplicateName?: boolean;
/**
* Re-index every repo in ~/.gitnexus/registry.json (#253). When set,
* [path], --name, and --allow-duplicate-name are all rejected — the
* batch is a fleet operation, not a single-repo command. Each entry
* is processed by spawning a child `gitnexus analyze <path>` so that
* state (LadybugDB handles, progress bar, monkey-patched console)
* never carries between iterations — if one repo crashes, the others
* proceed untouched. Same per-repo error tolerance as `clean --all`.
*/
all?: boolean;
}

/**
* `analyze --all` implementation (#253): spawn a child
* `gitnexus analyze <path>` for each entry in the global registry,
* inheriting stdio. Child-process isolation means:
* - each repo gets a fresh LadybugDB handle, progress bar, and heap
* - a crash in one repo cannot bring down sibling cleanups
* - the existing analyze flow (SIGINT, ensureHeap, progress-bar
* monkey-patching) is reused verbatim — no in-process state
* reset hazards
*
* Forwarded flags: --force, --embeddings, --skills, --skip-agents-md,
* --no-stats, --skip-git, -v. NOT forwarded: --name,
* --allow-duplicate-name (per-repo concepts; validated out above),
* [path] positional (ditto).
*
* Exit code: 0 when all entries succeeded or were cleanly skipped
* (missing-path entries). 1 when at least one entry's child exited
* non-zero — surfaces batch partial-failure to cron / CI.
*/
const analyzeAllBranch = async (options: AnalyzeOptions | undefined): Promise<void> => {

@magyargergo magyargergo Apr 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to be careful with this! Imagine having 100s of cloned repositories and somebody runs GitNexus with --all option and there's a possibility it may crash the computer because of CPU exhaustion. I'd implement a queue and depending on resource availability i'd start processing them. Also don't forget the pipeline itself also spawns subproceses. Maybe it would be faster to not to call the whole gitnexus but to call the ingestion pipeline with the necessary arguments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review — good catch, and I want to make sure I implement the right queue shape rather than guess and iterate.

Current state (for calibration)

The current implementation is strictly sequentialspawnSync blocks the for-loop until each child exits (analyze.ts:190–198), so only ONE analyze runs at any moment. Pipeline-internal parse workers are already CPU-bounded inside each child. That said, your concern about sustained load over 100s of repos is still valid — even sequential, that's hours of 100% CPU with no graceful pause.

Which queue shape do you have in mind?

A few options with tradeoffs; happy to implement whichever you prefer:

(a) Throttled sequential with resource check. Stay 1-at-a-time, but check os.loadavg() between children and pause if load > threshold. Downside: os.loadavg() returns [0, 0, 0] on Windows, so the gate is Unix-only unless we proxy through something else.

(b) --concurrency <n> with default 1. Opt-in parallelism for users who know their machine can handle it. Default 1 = current sequential behaviour. Downside: contradicts the CPU-exhaustion framing — parallel makes CPU pressure worse, not better — so I don't think this is what you meant, flagging for disambiguation.

(c) Confirmation gate + estimated duration. Print "About to re-index N repos sequentially. Estimated time: ~{N × avg}s. Pass --yes to confirm." and require -y, --yes for non-interactive runs. Same pattern as clean --all --force. Makes the resource cost visible before the user commits to it.

(d) Something else — happy to build it if you have a specific shape in mind.

The pipeline-direct suggestion

On "call the ingestion pipeline with the necessary arguments" — I deliberately went child-process-per-repo to keep LadybugDB handles, progress bar, SIGINT, and the monkey-patched console cleanly isolated per repo. Calling runFullAnalysis in-process would save ~1–2s process-startup per repo but means (i) a crash in one repo aborts the batch, (ii) state-reset hazards between iterations we'd need to prove idempotent. Happy to revisit if the queue design ends up needing in-process for some other reason — but I'd rather not bundle it in unless there's a concrete motivation.

Will hold on code changes until you point at the shape you want. 🙏

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very primitive but could be a good starting point

import si from 'systeminformation';

const CPU_THRESHOLD = 80;
const MEM_THRESHOLD = 85;

setInterval(async () => {
  const [load, mem] = await Promise.all([
    si.currentLoad(),
    si.mem(),
  ]);

  const cpu = load.currentLoad;                  // %
  const memUsed = (mem.used / mem.total) * 100; // %

  const shouldThrottle = cpu > CPU_THRESHOLD || memUsed > MEM_THRESHOLD;

  console.log({
    cpu: cpu.toFixed(1) + '%',
    mem: memUsed.toFixed(1) + '%',
    shouldThrottle,
  });
}, 1000);

More information about the npm package: https://www.npmjs.com/package/systeminformation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the concrete pointer — implemented in `1fb6bc36` using `systeminformation` exactly as you sketched.

What shipped

Defaults match your sketch 1:1: 80% CPU / 85% memory thresholds, 1-second poll interval, simple `cpu > CPU_THRESHOLD || memUsed > MEM_THRESHOLD` predicate.

Shape: between each `--all` child spawn, the batch polls CPU + memory. If either metric is above threshold, it prints `⏸ Throttling — CPU X.X% / mem Y.Y% (thresholds 80% / 85%) — waiting before [i/N]...` and re-polls every second. When both drop below threshold, it prints `▶ Resuming — CPU X.X% / mem Y.Y%` and spawns the next child.

No max-wait timeout by design. A host pinned above threshold is exactly the case you asked us to protect, so bailing after some arbitrary deadline would silently defeat the safety. Ctrl-C is the user's graceful exit.

Escape hatches:

  • `--no-throttle` CLI flag for dedicated CI / build agents that accept the full resource cost
  • `GITNEXUS_THROTTLE_CPU` / `GITNEXUS_THROTTLE_MEM` env vars for advanced tuning without code changes

On broken metrics provider (e.g. `systeminformation` platform issue): the batch falls through WITHOUT throttling rather than hanging indefinitely. Unit-tested.

Pipeline-direct suggestion

Kept child-process-per-repo. The isolation story (each repo gets fresh LadybugDB handles, progress bar, monkey-patched console, SIGINT handler) is what lets one repo crash without corrupting the batch. Happy to revisit if you want to swap in-process for a specific reason later — but since your main concern (resource pressure) is now addressed at the orchestration layer, I'd rather not bundle that bigger change in here.

Tests

  • 15 new unit tests on `resource-throttle.ts` covering the predicate (both-under / CPU-only / mem-only / both-over / boundary), polling trajectory (idle-on-first-poll, multi-poll-then-clear), provider-throw tolerance, and env-var parsing (valid / NaN / 0 / negative / >100 / empty / fractional).
  • 1 new integration test locks in `--no-throttle` bypass: sets thresholds to 1% (which would hang indefinitely without bypass) and asserts the batch completes AND the Throttling messaging is absent from output.

Local verification on the final commit:

  • `tsc --noEmit` clean
  • `resource-throttle.test.ts` 15/15
  • Full unit suite 4216 pass (2 pre-existing git-utils env failures unchanged, unrelated — exist on pristine upstream main)
  • `cli-e2e.test.ts -t "analyze --all|remove|clean --all|analyze --name|no-throttle"` 11/11 in 178s

CI running.

const entries = await listRegisteredRepos();
if (entries.length === 0) {
console.log('\n No indexed repositories found.');
console.log(' Run `gitnexus analyze <path>` in a repo to index it first.\n');
return;
}

console.log(`\n GitNexus Analyzer — batch mode (${entries.length} repo(s))\n`);

// Resolve the CLI entrypoint once so each child spawn re-uses it. Use
// the same path the current process was launched with — that way
// `dist/` vs source-via-tsx works identically in CI, dev, and
// end-user installs. `process.argv[1]` is the path to index.js (or
// the tsx'd index.ts during tests).
const cliEntry = process.argv[1];
if (!cliEntry) {
console.error('Error: could not determine gitnexus CLI path from process.argv[1].');
process.exitCode = 1;
return;
}

// Build the forwarded-flag list once — same for every child. This
// deliberately excludes --all itself (prevents infinite recursion),
// --name, --allow-duplicate-name (per-repo), and the [path]
// positional (each child gets the registry entry's path instead).
const forwarded: string[] = [];
if (options?.force) forwarded.push('--force');
if (options?.embeddings) forwarded.push('--embeddings');
if (options?.skills) forwarded.push('--skills');
if (options?.skipAgentsMd) forwarded.push('--skip-agents-md');
if (options?.noStats) forwarded.push('--no-stats');
if (options?.skipGit) forwarded.push('--skip-git');
if (options?.verbose) forwarded.push('--verbose');

let succeeded = 0;
let skipped = 0;
let failed = 0;
const failedNames: string[] = [];

// Resource-aware throttle config (#1010 review — @magyargergo).
// Defaults match the reviewer's sketch (80% CPU / 85% mem); env vars
// let operators tune without code changes. Non-bypassable by design —
// per review round 2, `--all` must not ship an agent-accessible
// override because "resource exhaustion comes with a great cost". If
// an operator configures thresholds so high that the safeguard is
// effectively disabled, we surface a one-time warning at batch start
// ("warning on overusing resources") rather than a silent bypass.
const throttleThresholds = readThresholdsFromEnv();
warnIfThresholdsRisky(throttleThresholds);

for (let i = 0; i < entries.length; i++) {
const entry = entries[i];

// Pace the batch to host headroom BEFORE announcing the repo. That
// way the "[i/N] Analyzing" heading always reflects work that is
// actually starting, not a repo queued behind a throttle wait.
await waitForResourceAvailability({
thresholds: throttleThresholds,
onThrottling: (snapshot) => {
console.warn(
` ⏸ Throttling — CPU ${snapshot.cpuPct.toFixed(1)}% / mem ${snapshot.memPct.toFixed(
1,
)}% (thresholds ${throttleThresholds.cpuPct}% / ${throttleThresholds.memPct}%) — ` +
`waiting before [${i + 1}/${entries.length}]...`,
);
},
onResumed: (snapshot) => {
console.log(
` ▶ Resuming — CPU ${snapshot.cpuPct.toFixed(1)}% / mem ${snapshot.memPct.toFixed(1)}%`,
);
},
});

console.log(`\n [${i + 1}/${entries.length}] Analyzing: ${entry.name} (${entry.path})`);

// Skip entries whose repo has been deleted externally. Same
// error-tolerance principle as `clean --all`: a stale registry
// entry should not halt the batch.
try {
if (!fsSync.existsSync(entry.path)) {
console.warn(` ⚠ Skipped (path no longer exists): ${entry.path}`);
skipped++;
continue;
}
} catch (err) {
console.warn(` ⚠ Skipped (stat error): ${entry.name}: ${(err as Error).message}`);
skipped++;
continue;
}

// Spawn a child `gitnexus analyze <path>` with the forwarded flags
// and inherited stdio so the child's own progress bar / logs flow
// through naturally.
//
// Propagating `process.execArgv` is critical for the test suite
// (and any other tsx-loaded invocation): execArgv holds Node-level
// flags like `--import <tsx-loader-url>` that aren't part of argv.
// Without them, the child can't load .ts sources and dies with
// `ERR_UNKNOWN_FILE_EXTENSION`. In production (npm install of the
// compiled package), execArgv is typically empty, so this is a
// no-op — but it makes the CLI equally usable under tsx/tsnode dev
// setups and in vitest-spawned integration tests.
const result = spawnSync(
process.execPath,
[...process.execArgv, cliEntry, 'analyze', entry.path, ...forwarded],
{
stdio: 'inherit',
env: process.env,
},
);

if (result.status === 0) {
succeeded++;
} else {
failed++;
failedNames.push(entry.name);
const reason =
result.signal !== null
? `signal ${result.signal}`
: result.error
? (result.error as Error).message
: `exit code ${result.status}`;
console.error(` ✗ Failed (${reason}): ${entry.name}`);
}
}

console.log(
`\n Summary: ${succeeded} succeeded, ${skipped} skipped, ${failed} failed.` +
(failed > 0 ? `\n Failed: ${failedNames.join(', ')}\n` : '\n'),
);

// Exit non-zero on any failure so cron / CI pick up partial-failure
// conditions. Skipped entries (missing paths) are NOT failures —
// they're just registry self-heal candidates that `list --validate`
// would clean up on next read.
if (failed > 0) process.exitCode = 1;
};

export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOptions) => {
if (ensureHeap()) return;

if (options?.verbose) {
process.env.GITNEXUS_VERBOSE = '1';
}

// ── `analyze --all` branch (#253) ──────────────────────────────────
//
// Validate mutual exclusions up-front so the user sees a clear error
// BEFORE any registry or filesystem work happens. Each flag is
// incompatible with --all because it's inherently per-repo:
// - [path] → --all iterates the registry; the user
// can't also pin a single path
// - --name <alias> → an alias targets one repo
// - --allow-duplicate-name → same (registry-collision semantics)
if (options?.all) {
const violations: string[] = [];
if (inputPath) violations.push('[path] positional argument');
if (options.name !== undefined) violations.push('--name <alias>');
if (options.allowDuplicateName) violations.push('--allow-duplicate-name');
if (violations.length > 0) {
console.error(
`Error: --all cannot be combined with ${violations.join(', ')}. ` +
`--all re-indexes every registered repo; these flags target a single repo.`,
);
process.exitCode = 1;
return;
}
return analyzeAllBranch(options);
}

console.log('\n GitNexus Analyzer\n');

let repoPath: string;
Expand Down
14 changes: 13 additions & 1 deletion gitnexus/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,22 @@ program
'Register this repo even if another path already uses the same --name alias. ' +
'Leaves `-r <name>` ambiguous for the two paths; use -r <path> to disambiguate.',
)
.option(
'--all',
'Re-index every repo registered in ~/.gitnexus/registry.json. ' +
'Mirrors `clean --all`. Cannot be combined with [path], --name, or --allow-duplicate-name. ' +
'Skips entries whose path no longer exists on disk; failures in one repo do not halt the batch. ' +
'Pauses between repos when CPU > 80% or memory > 85% (non-bypassable safeguard).',
)
.option('-v, --verbose', 'Enable verbose ingestion warnings (default: false)')
.addHelpText(
'after',
'\nEnvironment variables:\n GITNEXUS_NO_GITIGNORE=1 Skip .gitignore parsing (still reads .gitnexusignore)',
'\nEnvironment variables:\n' +
' GITNEXUS_NO_GITIGNORE=1 Skip .gitignore parsing (still reads .gitnexusignore)\n' +
' GITNEXUS_THROTTLE_CPU With --all: CPU %% threshold above which we pause (default 80). ' +
'A warning is printed when raised above 90%% (resource-exhaustion risk).\n' +
' GITNEXUS_THROTTLE_MEM With --all: memory %% threshold above which we pause (default 85). ' +
'A warning is printed when raised above 90%% (resource-exhaustion risk).',
)
.action(createLazyAction(() => import('./analyze.js'), 'analyzeCommand'));

Expand Down
Loading
Loading