GitNexus/gitnexus/src/cli/tool.ts
Malik 859e4b75a4
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
fix(cli): --limit i18n, 0/negative guard, and correct truncation paths (#2310)
* fix: add --limit i18n, negative guard, correct property paths, and zh-CN translations

- Add i18n keys for context/impact/cypher/detect-changes --limit options
- Add zh-CN translations for all 4 --limit option descriptions
- Add Math.max(0, parseInt()) guard to prevent negative --limit
- Fix ALL property path mismatches discovered by audit:
  - context: callers/callees → incoming.calls/outgoing.calls+accesses
  - impact: upstream/downstream → affected_processes/affected_modules/byDepth
  - cypher: rows → row_count cap (rows embedded in markdown string)
  - detect-changes: affected_flows → affected_processes
- Change query command from required to optional positional arg with -q alias
- Update @ladybugdb/core from ^0.16.1 to ^0.17.1
- Update typescript from ^5.4.5 to ^5.9.3

* test: add E2E tests for --limit flag across all 5 CLI commands

Tests context, impact, cypher, detect-changes, and query with
--limit 1, baseline comparison, and --limit 0 (falsy/no-op).

detect-changes output is formatted text (not JSON), so those
tests count symbol lines matching 'Type name -> filePath' pattern.

14 tests, all passing. No regressions in 6455 existing tests.

* fix: address Copilot review feedback on --limit guards

- Add Math.max(0, ...) guard to queryCommand limit parsing
- Change if(limit) to if(limit !== undefined) in all 5 commands
  (prevents --limit 0 from being treated as falsy/no-op)
- Make queryText parameter optional (Commander may pass undefined)
- Fix usage error strings: --search to -q, --query (en + zh-CN)

* chore(autofix): apply prettier + eslint fixes via /autofix command

* fix(cli): centralize --limit parsing, slice cypher markdown, fix usage text

Address PR review feedback on --limit handling:

- Add a shared parseLimit() helper (Number.isInteger(n) && n > 0), used by all
  5 tool commands. Non-numeric / 0 / negative --limit now means "no limit"
  instead of the `options.limit ? Math.max(0, parseInt(...)) : undefined` path,
  where a string like "abc" is truthy and yields NaN -> slice(0, NaN) -> the
  guardrail commands (impact/context/detect-changes) silently emptied results
  with exit 0.
- cypher: slice the markdown table to --limit data rows so the reported
  row_count matches what is actually printed (was capping row_count while
  printing every row).
- Fix query usage string: [search_query] (optional positional) and
  `--query <text>` invocation form, not the option-definition
  `-q, --query <search_query>` syntax (en + zh-CN).
- Add an E2E regression test for non-numeric --limit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(cli): escape newlines in cypher markdown cells

A multi-line cell value (e.g. a symbol's `content`) was rendered with raw
newlines via String(v), so one logical row spanned multiple physical lines.
That corrupts the markdown table and breaks `cypher --limit`'s line-based
slice (it kept the wrong number of rows, often zero, while row_count
over-claimed). Collapse newlines in formatCypherAsMarkdown so one physical
line == one row; the existing CLI slice is now correct and the pre-existing
un---limited corruption is fixed too. (#2310 review)

* test(cli): de-vacuum the --limit truncation tests

The truncation it()s used the repo-banned vacuous-pass pattern (early-return
on status===null, assertions guarded by if(Array.isArray), bounds-only
toBeLessThanOrEqual — DoD.md:82) against `validateInput`, which has only 1
caller, so context/impact/query --limit 1 compared 1>=1 and stayed green even
if the slice were deleted. Rewrite with unconditional, exact assertions and
target `logMessage` (2 callers, 4 processes) so the no-limit baseline truly
exceeds the limit; detect-changes now mutates two real function bodies (two
changed symbols). Adds a multi-line-cell cypher --limit regression. (#2310)

* test(ci): run cli-limit-e2e in the cross-platform matrix

The --limit E2E suite spawns the real CLI (child_process) but was not in
SPAWN_CLI, so it ran only on Ubuntu — the cross-platform check only fails on
listed-but-missing files, not the reverse (TESTING.md §Cross-platform). Register
it so the --limit regression guard also runs on Windows/macOS, where path
separators, CRLF and the formatted-output arrow differ. (#2310)

* fix(cli): document impact --limit affected-list cap, drop dead byDepth re-slice

`impact --limit` also caps affected_processes/modules, but the help only
mentioned the per-depth cap — so JSON consumers reading the affected lists got
a silently-truncated array. Update en + zh-CN + the command description to say
so. Also remove the client-side byDepth re-slice: the backend already
paginates byDepth to the same limit (paginationLimit = clamp(limit,1,10000),
offset applied backend-side), so the client slice was a guaranteed no-op. (#2310)

* fix(cli): reconcile detect-changes --limit summary, list, and overflow

formatDetectChangesResult computed the "... and N more" overflow from the
already---limit-sliced array length, so under `--limit` the header (true
summary total), the listed rows, and the marker disagreed — e.g. "2 symbols"
in the header but a list of 1 with no marker. Base the overflow on the true
summary.changed_count / affected_count instead, and add the same marker to the
affected-processes list, so header + list + marker stay consistent. (#2310)

* feat(cli): add -l shorthand to impact --limit

The PR added the -l alias to context/cypher/detect-changes but left impact on
the long --limit only, so `impact -l 5` errored while `context -l 5` worked.
Add -l for parity and update the help-i18n OPTION_DESCRIPTION_KEYS key to the
new `-l, --limit <n>` flag string so the description still resolves. (#2310)

* fix(cli): bound all context --limit array categories

context --limit sliced only incoming.calls / outgoing.calls / outgoing.accesses
/ processes, leaving the other relType buckets unbounded — notably
incoming.accesses (bounded on outgoing but not incoming) plus imports/extends/
uses/… and typed_properties. Replace the hardcoded slices with a generic loop
over every array-valued bucket under incoming/outgoing, plus typed_properties
and processes, so --limit caps the whole context payload. (#2310)

* refactor(cli): parse --offset with a parseLimit-style helper

impactCommand parsed --offset with the legacy parseInt/Number.isFinite idiom
while --limit had moved to parseLimit, leaving two parsing styles side by side.
Add a sibling parseOffset helper (non-negative — offset 0 is valid) and use it,
so both options share one idiom; as a bonus it now rejects negative/fractional
offsets instead of forwarding them to the backend. (#2310)

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-01 19:16:36 +01:00

429 lines
14 KiB
TypeScript

/**
* Direct CLI Tool Commands
*
* Exposes GitNexus tools (query, context, impact, cypher, check) as direct CLI commands.
* Bypasses MCP entirely — invokes LocalBackend directly for minimal overhead.
*
* Usage:
* gitnexus query "authentication flow"
* gitnexus context --name "validateUser"
* gitnexus impact --target "AuthService" --direction upstream
* gitnexus cypher "MATCH (n:Function) RETURN n.name LIMIT 10"
*
* Note: Output goes to stdout via fs.writeSync(fd 1), bypassing LadybugDB's
* native module which captures the Node.js process.stdout stream during init.
* See the output() function for details (#324).
*/
import { writeSync } from 'node:fs';
import { LocalBackend, VALID_NODE_LABELS } from '../mcp/local/local-backend.js';
import { cliErrorKey, cliWarnKey } from './cli-message.js';
import { formatDetectChangesResult } from './detect-changes-format.js';
let _backend: LocalBackend | null = null;
async function getBackend(): Promise<LocalBackend> {
if (_backend) return _backend;
_backend = new LocalBackend();
const ok = await _backend.init();
if (!ok) {
cliErrorKey('tool.noIndexed');
process.exit(1);
}
return _backend;
}
/**
* Write tool output to stdout using low-level fd write.
*
* LadybugDB's native module captures Node.js process.stdout during init,
* but the underlying OS file descriptor 1 (stdout) remains intact.
* By using fs.writeSync(1, ...) we bypass the Node.js stream layer
* and write directly to the real stdout fd (#324).
*
* Falls back to stderr if the fd write fails (e.g., broken pipe).
*/
function output(data: any): void {
const text = typeof data === 'string' ? data : JSON.stringify(data, null, 2);
try {
writeSync(1, text + '\n');
} catch (err: any) {
if (err?.code === 'EPIPE') {
// Consumer closed the pipe (e.g., `gitnexus cypher ... | head -1`)
// Exit cleanly per Unix convention
process.exit(0);
}
// Fallback: stderr (previous behavior, works on all platforms)
process.stderr.write(text + '\n');
}
}
/**
* Parse a `--limit` CLI option into a positive row cap, or `undefined` when the
* flag is absent, non-numeric, zero, or negative.
*
* Treating invalid / 0 / negative input as "no limit" — rather than the old
* `options.limit ? Math.max(0, parseInt(...)) : undefined` path, where a string
* like `"abc"` is truthy and yields `NaN`, then `slice(0, NaN)` silently EMPTIES
* the result with exit 0 — keeps the guardrail commands (impact / context /
* detect-changes) honest: a bad `--limit` shows everything, never nothing.
*/
function parseLimit(raw: string | undefined): number | undefined {
if (raw === undefined) return undefined;
const n = Number(raw);
return Number.isInteger(n) && n > 0 ? n : undefined;
}
/**
* Parse an `--offset` CLI option into a non-negative pagination start, or
* `undefined` when the flag is absent or invalid. Mirrors {@link parseLimit};
* offset `0` is valid ("start at the beginning"), so the guard is `>= 0`.
*/
function parseOffset(raw: string | undefined): number | undefined {
if (raw === undefined) return undefined;
const n = Number(raw);
return Number.isInteger(n) && n >= 0 ? n : undefined;
}
export async function queryCommand(
queryText: string | undefined,
options?: {
query?: string;
repo?: string;
branch?: string;
context?: string;
goal?: string;
limit?: string;
content?: boolean;
},
): Promise<void> {
const resolvedQuery = queryText?.trim() || options?.query?.trim();
if (!resolvedQuery) {
cliErrorKey('tool.usage.query');
process.exit(1);
}
const backend = await getBackend();
const result = await backend.callTool('query', {
// #2175: canonical param is search_query; the backend still accepts legacy "query".
search_query: resolvedQuery,
task_context: options?.context,
goal: options?.goal,
limit: parseLimit(options?.limit),
include_content: options?.content ?? false,
repo: options?.repo,
branch: options?.branch,
});
output(result);
}
export async function contextCommand(
name: string,
options?: {
repo?: string;
branch?: string;
file?: string;
uid?: string;
limit?: string;
content?: boolean;
},
): Promise<void> {
// Reject a `--`-prefixed uid swallowed from a following flag (see impactCommand).
if (options?.uid?.startsWith('--')) {
cliErrorKey('tool.usage.context');
process.exit(1);
}
if (!name?.trim() && !options?.uid) {
cliErrorKey('tool.usage.context');
process.exit(1);
}
const limit = parseLimit(options?.limit);
const backend = await getBackend();
const result = await backend.callTool('context', {
name: name || undefined,
uid: options?.uid,
file_path: options?.file,
include_content: options?.content ?? false,
repo: options?.repo,
branch: options?.branch,
});
if (limit !== undefined) {
// Bound every array-valued category under incoming/outgoing (calls, accesses,
// imports, extends, uses, …) — categorize() buckets by relType, so the prior
// hardcoded calls/accesses missed the rest (e.g. incoming.accesses) — plus
// typed_properties and processes, so --limit caps the whole context payload.
for (const dir of [result.incoming, result.outgoing] as Array<
Record<string, unknown> | undefined
>) {
if (!dir) continue;
for (const key of Object.keys(dir)) {
const bucket = dir[key];
if (Array.isArray(bucket)) dir[key] = bucket.slice(0, limit);
}
}
if (Array.isArray(result.typed_properties))
result.typed_properties = result.typed_properties.slice(0, limit);
if (Array.isArray(result.processes)) result.processes = result.processes.slice(0, limit);
}
output(result);
}
export async function impactCommand(
target?: string,
options?: {
direction?: string;
mode?: string;
line?: string;
repo?: string;
branch?: string;
uid?: string;
file?: string;
kind?: string;
depth?: string;
includeTests?: boolean;
limit?: string;
offset?: string;
summaryOnly?: boolean;
},
): Promise<void> {
// A `--`-prefixed uid means Commander swallowed a following flag as the uid
// value (e.g. `impact --uid --file x` → uid === '--file'). Reject it rather
// than forwarding a garbage uid that would silently resolve to not-found.
if (options?.uid?.startsWith('--')) {
cliErrorKey('tool.usage.impact');
process.exit(1);
}
// Target is an optional positional: a uid alone is enough to resolve (parity
// with `context [name]`). Only error when neither a target nor a uid is given.
if (!target?.trim() && !options?.uid) {
cliErrorKey('tool.usage.impact');
process.exit(1);
}
// Soft-validate --kind: an unknown kind is a no-op hint (the backend scores
// it but it matches nothing), so warn and proceed rather than rejecting —
// parity with the lenient MCP surface and forward-compatible with new labels.
if (options?.kind && !VALID_NODE_LABELS.has(options.kind)) {
cliWarnKey('tool.warn.unknownKind', { kind: options.kind });
}
try {
const backend = await getBackend();
const parsedLimit = parseLimit(options?.limit);
const parsedOffset = parseOffset(options?.offset);
// `--line` is a PDG-only statement anchor (1-based source line). Parse it to
// an integer when provided and thread it ONLY when present, so the backend's
// line-without-pdg / non-positive-integer validation fires on the real value
// rather than on a silently-dropped flag. A non-numeric `--line` parses to
// NaN, which the backend rejects as a non-positive integer (loud, not silent).
const parsedLine = options?.line !== undefined ? parseInt(options.line, 10) : undefined;
const result = await backend.callTool('impact', {
target: target || undefined,
target_uid: options?.uid,
file_path: options?.file,
kind: options?.kind,
direction: options?.direction || 'upstream',
// Forward the engine selector; backend validates the enum (callgraph/pdg)
// and treats the default 'callgraph' identically to an omitted mode.
mode: options?.mode,
// PDG-only statement anchor — forwarded only when --line was given.
...(parsedLine !== undefined ? { line: parsedLine } : {}),
maxDepth: options?.depth ? parseInt(options.depth, 10) : undefined,
includeTests: options?.includeTests ?? false,
repo: options?.repo,
branch: options?.branch,
limit: parsedLimit,
offset: parsedOffset,
summaryOnly: options?.summaryOnly ?? undefined,
});
// Client-side cap of the affected-list payload to --limit (parity with the
// other tool commands). The backend already paginates byDepth per level to
// the same limit, so byDepth needs no client-side re-slice.
if (parsedLimit !== undefined) {
if (Array.isArray(result.affected_processes))
result.affected_processes = result.affected_processes.slice(0, parsedLimit);
if (Array.isArray(result.affected_modules))
result.affected_modules = result.affected_modules.slice(0, parsedLimit);
}
output(result);
} catch (err: unknown) {
// Belt-and-suspenders: catch infrastructure failures (getBackend, callTool transport)
// The backend's impact() already returns structured errors for graph query failures
output({
error:
(err instanceof Error ? err.message : String(err)) || 'Impact analysis failed unexpectedly',
target: { name: target },
direction: options?.direction || 'upstream',
suggestion: 'Try reducing --depth or using gitnexus context <symbol> as a fallback',
});
process.exit(1);
}
}
export async function cypherCommand(
query: string,
options?: {
repo?: string;
branch?: string;
limit?: string;
},
): Promise<void> {
if (!query?.trim()) {
cliErrorKey('tool.usage.cypher');
process.exit(1);
}
const limit = parseLimit(options?.limit);
const backend = await getBackend();
const result = await backend.callTool('cypher', {
// #2175: canonical param is statement; the backend still accepts legacy "query".
statement: query,
repo: options?.repo,
branch: options?.branch,
});
if (limit !== undefined) {
if (Array.isArray(result)) {
// Non-tabular result: a raw row array.
result.splice(limit);
} else if (result && typeof result === 'object' && typeof result.row_count === 'number') {
// Tabular result: { markdown, row_count }. The markdown is a table built as
// [header, separator, ...dataRows].join('\n'), so slice it to `limit` data
// rows (keeping the 2 header lines) and report a row_count that matches what
// is actually printed — otherwise `--limit 2` over 50 rows prints all 50 but
// claims row_count: 2.
if (typeof result.markdown === 'string' && result.row_count > limit) {
result.markdown = result.markdown
.split('\n')
.slice(0, 2 + limit)
.join('\n');
}
result.row_count = Math.min(result.row_count, limit);
}
}
output(result);
}
export async function detectChangesCommand(options?: {
scope?: string;
baseRef?: string;
repo?: string;
branch?: string;
limit?: string;
}): Promise<void> {
const limit = parseLimit(options?.limit);
const backend = await getBackend();
const result = await backend.callTool('detect_changes', {
scope: options?.scope || 'unstaged',
base_ref: options?.baseRef,
repo: options?.repo,
branch: options?.branch,
});
if (limit !== undefined) {
if (Array.isArray(result.changed_symbols))
result.changed_symbols = result.changed_symbols.slice(0, limit);
if (Array.isArray(result.affected_processes))
result.affected_processes = result.affected_processes.slice(0, limit);
}
output(formatDetectChangesResult(result));
}
export async function checkCommand(options?: {
cycles?: boolean;
json?: boolean;
repo?: string;
branch?: string;
}): Promise<void> {
if (!options?.cycles) {
process.stderr.write('Usage: gitnexus check --cycles [--json]\n');
process.exitCode = 1;
return;
}
try {
const backend = await getBackend();
const result = await backend.callTool('check', {
cycles: true,
repo: options.repo,
branch: options.branch,
});
if (result?.error) {
output(result);
process.exitCode = 1;
return;
}
if (options.json) {
output(result);
} else if (result.cycleCount === 0) {
output('No circular imports found.');
} else {
output(
result.cycles.map((cycle: { files: string[] }) => cycle.files.join(' -> ')).join('\n'),
);
}
if (result.cycleCount > 0) process.exitCode = 1;
} catch (error) {
output({ error: error instanceof Error ? error.message : String(error) });
process.exitCode = 1;
}
}
export async function traceCommand(
from?: string,
to?: string,
options?: {
fromUid?: string;
fromFile?: string;
toUid?: string;
toFile?: string;
depth?: string;
repo?: string;
branch?: string;
includeTests?: boolean;
},
): Promise<void> {
if (options?.fromUid?.startsWith('--') || options?.toUid?.startsWith('--')) {
cliErrorKey('tool.usage.trace');
process.exit(1);
}
if ((!from?.trim() && !options?.fromUid) || (!to?.trim() && !options?.toUid)) {
cliErrorKey('tool.usage.trace');
process.exit(1);
}
// Reject a non-numeric / non-positive --depth up front rather than forwarding
// NaN (which the backend would silently treat as the default).
if (options?.depth !== undefined) {
const parsedDepth = Number(options.depth);
if (!Number.isInteger(parsedDepth) || parsedDepth < 1) {
cliErrorKey('tool.usage.trace');
process.exit(1);
}
}
try {
const backend = await getBackend();
const result = await backend.callTool('trace', {
from: from || undefined,
from_uid: options?.fromUid,
from_file: options?.fromFile,
to: to || undefined,
to_uid: options?.toUid,
to_file: options?.toFile,
maxDepth: options?.depth ? parseInt(options.depth, 10) : undefined,
includeTests: options?.includeTests ?? false,
repo: options?.repo,
branch: options?.branch,
});
output(result);
} catch (err: unknown) {
output({
status: 'error',
error:
(err instanceof Error ? err.message : String(err)) || 'Trace analysis failed unexpectedly',
from: { name: from },
to: { name: to },
suggestion:
'Try gitnexus context <symbol> to see connections, or check if an interface bridges them.',
});
process.exit(1);
}
}