scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-08-28 05:25:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

Merge branch 'main' into fix/2606-global-ignore-file

Browse source
This commit is contained in:
Gergő Magyar 2026-07-21 18:39:22 +01:00 • committed by GitHub
commit 8a67acb9dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 333 additions and 107 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

201
gitnexus/src/mcp/local/local-backend.ts
View file

@ -4361,44 +4361,31 @@ export class LocalBackend {
return { error: 'New name is the same as the current name.' };
}
// Step 2: Collect edits from graph (high confidence)
const changes = new Map<string, { file_path: string; edits: any[] }>();
const addEdit = (
filePath: string,
line: number,
oldText: string,
newText: string,
confidence: string,
) => {
if (!changes.has(filePath)) {
changes.set(filePath, { file_path: filePath, edits: [] });
}
changes.get(filePath)!.edits.push({ line, old_text: oldText, new_text: newText, confidence });
// Steps 2+3: Determine the set of files the apply step will rewrite, then
// enumerate every occurrence in each. The apply step (Step 4) does a
// whole-file `\boldName\b` global replace on every file in `changes`, so the
// reported edit list MUST enumerate every matching line in every such file —
// otherwise the preview under-reports what lands, and the same partial list
// comes back after apply (#2605). Building `changes` from one file set makes
// the preview enumerate exactly the files the apply loop rewrites, using the
// same word-boundary regex. (This is per-call consistency; the apply loop
// still re-reads each file, so an external write landing between preview and
// apply is a pre-existing gap this method does not lock against.)
type RenameEdit = {
line: number;
old_text: string;
new_text: string;
confidence: 'graph' | 'text_search';
};
const escapedOldName = oldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
// The definition itself
if (sym.filePath && sym.startLine) {
try {
const content = await fs.readFile(assertSafePath(sym.filePath), 'utf-8');
const lines = content.split('\n');
const lineIdx = sym.startLine - 1;
if (lineIdx >= 0 && lineIdx < lines.length && lines[lineIdx].includes(oldName)) {
const defRegex = new RegExp(
`\\b${oldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\b`,
'g',
);
addEdit(
sym.filePath,
sym.startLine,
lines[lineIdx].trim(),
lines[lineIdx].replace(defRegex, new_name).trim(),
'graph',
);
}
} catch (e) {
logQueryError('rename:read-definition', e);
}
// Classify each file to rewrite by how it was discovered. Definition and
// graph-ref files carry graph confidence; files found only by text search
// carry text_search confidence. A graph-classified file is never downgraded.
const fileConfidence = new Map<string, 'graph' | 'text_search'>();
if (sym.filePath) {
fileConfidence.set(sym.filePath, 'graph');
}
// All incoming refs from graph (callers, importers, etc.)
@ -4408,44 +4395,13 @@ export class LocalBackend {
...(lookupResult.incoming.extends || []),
...(lookupResult.incoming.implements || []),
];
let graphEdits = changes.size > 0 ? 1 : 0; // count definition edit
for (const ref of allIncoming) {
if (!ref.filePath) continue;
try {
const content = await fs.readFile(assertSafePath(ref.filePath), 'utf-8');
const lines = content.split('\n');
for (let i = 0; i < lines.length; i++) {
if (lines[i].includes(oldName)) {
addEdit(
ref.filePath,
i + 1,
lines[i].trim(),
lines[i]
.replace(
new RegExp(`\\b${oldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\b`, 'g'),
new_name,
)
.trim(),
'graph',
);
graphEdits++;
break; // one edit per file from graph refs
}
}
} catch (e) {
logQueryError('rename:read-ref', e);
if (ref.filePath) {
fileConfidence.set(ref.filePath, 'graph');
}
}
// Step 3: Text search for refs the graph might have missed
let astSearchEdits = 0;
const graphFiles = new Set(
[sym.filePath, ...allIncoming.map((r) => r.filePath)].filter(Boolean),
);
// Simple text search across the repo for the old name (in files not already covered by graph)
// Text search for files the graph might have missed entirely.
try {
const { execFileSync } = await import('child_process');
const rgArgs = [
@ -4472,67 +4428,98 @@ export class LocalBackend {
for (const file of files) {
const normalizedFile = file.replace(/\\/g, '/').replace(/^\.\//, '');
if (graphFiles.has(normalizedFile)) continue; // already covered by graph
try {
const content = await fs.readFile(assertSafePath(normalizedFile), 'utf-8');
const lines = content.split('\n');
const regex = new RegExp(`\\b${oldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\b`, 'g');
for (let i = 0; i < lines.length; i++) {
regex.lastIndex = 0;
if (regex.test(lines[i])) {
regex.lastIndex = 0;
addEdit(
normalizedFile,
i + 1,
lines[i].trim(),
lines[i].replace(regex, new_name).trim(),
'text_search',
);
astSearchEdits++;
}
}
} catch (e) {
logQueryError('rename:text-search-read', e);
// Never downgrade a graph-classified file to text_search.
if (!fileConfidence.has(normalizedFile)) {
fileConfidence.set(normalizedFile, 'text_search');
}
}
} catch (e) {
logQueryError('rename:ripgrep', e);
}
// Step 4: Apply or preview
const allChanges = Array.from(changes.values());
const totalEdits = allChanges.reduce((sum, c) => sum + c.edits.length, 0);
// Enumerate every `\boldName\b` line in each file to rewrite, so the previewed
// file set is exactly the set the apply loop below rewrites. A file with no
// matching line is dropped (apply would write nothing to it). `wordTest`
// (non-global) probes each line; `wordReplace` (global) rewrites it and is
// reused by the apply loop — compiled once each rather than once per line,
// and one escaping formula serves both passes.
const wordTest = new RegExp(`\\b${escapedOldName}\\b`);
const wordReplace = new RegExp(`\\b${escapedOldName}\\b`, 'g');
const changes = new Map<string, { file_path: string; edits: RenameEdit[] }>();
for (const [filePath, confidence] of fileConfidence) {
try {
const content = await fs.readFile(assertSafePath(filePath), 'utf-8');
const lines = content.split('\n');
const edits: RenameEdit[] = [];
for (let i = 0; i < lines.length; i++) {
if (!wordTest.test(lines[i])) {
continue;
}
edits.push({
line: i + 1,
old_text: lines[i].trim(),
new_text: lines[i].replace(wordReplace, new_name).trim(),
confidence,
});
}
if (edits.length > 0) {
changes.set(filePath, { file_path: filePath, edits });
}
} catch (e) {
logQueryError('rename:enumerate', e);
}
}
// Step 4: Apply or preview.
const failedFiles: string[] = [];
if (!dry_run) {
// Apply edits to files
for (const change of allChanges) {
for (const change of changes.values()) {
try {
const fullPath = assertSafePath(change.file_path);
let content = await fs.readFile(fullPath, 'utf-8');
const regex = new RegExp(`\\b${oldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\b`, 'g');
content = content.replace(regex, new_name);
await fs.writeFile(fullPath, content, 'utf-8');
const content = await fs.readFile(fullPath, 'utf-8');
await fs.writeFile(fullPath, content.replace(wordReplace, new_name), 'utf-8');
} catch (e) {
// A swallowed write failure must not be reported as a full success
// (#2283): record the file so the result can degrade to 'partial'
// with the unwritten files listed, rather than masquerading as done.
// A swallowed write failure must not be reported as success (#2283):
// record the file so the result degrades to 'partial'.
logQueryError('rename:apply-edit', e);
failedFiles.push(change.file_path);
}
}
// A file whose write threw did not land, so drop its edits from the
// reported result — total_edits/changes must describe what actually
// reached disk, not what was attempted (#2605: the report matches reality
// even on partial failure). failed_files still names every dropped file.
for (const f of failedFiles) {
changes.delete(f);
}
}
// Counts derive from the reported set (dry-run: every enumerated file;
// apply: only files that landed), so the graph/text_search split always
// sums to total_edits and never overstates a partial apply.
const reported = Array.from(changes.values());
let graphEdits = 0;
let astSearchEdits = 0;
for (const change of reported) {
for (const edit of change.edits) {
if (edit.confidence === 'graph') {
graphEdits++;
} else {
astSearchEdits++;
}
}
}
return {
status: failedFiles.length > 0 ? 'partial' : 'success',
old_name: oldName,
new_name,
files_affected: allChanges.length,
total_edits: totalEdits,
files_affected: reported.length,
total_edits: graphEdits + astSearchEdits,
graph_edits: graphEdits,
text_search_edits: astSearchEdits,
changes: allChanges,
changes: reported,
applied: !dry_run,
...(failedFiles.length > 0 && { failed_files: failedFiles }),
};

239
gitnexus/test/unit/rename-edit-report.test.ts Normal file
View file

@ -0,0 +1,239 @@
/**
* Regression test for issue #2605: `rename` must report every edit it applies.
*
* The apply step does a whole-file `\boldName\b` global replace on each touched
* file, but the reported `changes`/`total_edits` were built from a partial
* enumeration that (a) recorded only the definition line, (b) recorded one edit
* per graph-ref file then broke, and (c) skipped text-search on any file already
* covered by the graph. When a private symbol's definition and all its call
* sites live in one file, only the definition line was reported (total_edits: 1)
* while apply rewrote every occurrence. These tests drive the single-file repro,
* a mixed graph/text_search multi-file rename, and a partial write failure, and
* assert the report matches what apply actually writes in each case.
*/
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { promises as fs } from 'node:fs';
import fsPromises from 'fs/promises';
import os from 'node:os';
import path from 'node:path';
// Prevent onnxruntime / native search adapters from loading at import time
// (mirrors test/unit/calltool-dispatch.test.ts). We drive the private rename()
// directly, so the graph/DB/embedding layers are never exercised.
vi.mock('../../src/core/search/bm25-index.js', () => ({
searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }),
}));
vi.mock('../../src/mcp/core/embedder.js', () => ({
embedQuery: vi.fn().mockResolvedValue([]),
getEmbeddingDims: vi.fn().mockReturnValue(384),
}));
// rename() shells out to `rg -l` to discover text-search files. Stub it so the
// ripgrep-discovery branch is deterministic and driveable (rg is not reliably
// on PATH inside the vitest worker). Default: no hits.
const { execFileSyncMock } = vi.hoisted(() => ({ execFileSyncMock: vi.fn(() => '') }));
vi.mock('child_process', async (importActual) => {
const actual = await importActual<typeof import('child_process')>();
return { ...actual, execFileSync: execFileSyncMock };
});
import { LocalBackend } from '../../src/mcp/local/local-backend.js';
type Incoming = {
calls: { filePath: string }[];
imports: { filePath: string }[];
extends: { filePath: string }[];
implements: { filePath: string }[];
};
const EMPTY_INCOMING: Incoming = { calls: [], imports: [], extends: [], implements: [] };
type RenameResult = {
status: string;
applied: boolean;
files_affected: number;
total_edits: number;
graph_edits: number;
text_search_edits: number;
changes: { file_path: string; edits: { line: number; confidence: string }[] }[];
failed_files?: string[];
};
// The #2605 repro: a private free fn with exactly 4 textual occurrences of
// `rename_target` — the definition, one production call, two test calls — all
// in the same file.
const RUST_SRC = `fn rename_target(x: u32) -> u32 {
x + 1
}
pub fn prod_call() -> u32 {
rename_target(1)
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn unit_one() {
assert_eq!(rename_target(1), 2);
}
#[test]
fn unit_two() {
assert_eq!(rename_target(2), 3);
}
}
`;
/** 1-based occurrence lines of `rename_target` in `src` — the ground truth the
* report must match. Computed (not hardcoded) so editing a fixture cannot
* silently desync the expectation. */
function occurrenceLines(src: string): number[] {
return src
.split('\n')
.map((line, i) => (/\brename_target\b/.test(line) ? i + 1 : 0))
.filter((n) => n > 0);
}
const OCCURRENCE_LINES = occurrenceLines(RUST_SRC);
/** Build a backend whose graph lookup returns the symbol (definition at
* src/lib.rs) with the given incoming refs. */
function stubbedBackend(incoming: Incoming = EMPTY_INCOMING): LocalBackend {
const backend = new LocalBackend();
vi.spyOn(
backend as unknown as { ensureInitialized: () => Promise<void> },
'ensureInitialized',
).mockResolvedValue(undefined);
vi.spyOn(backend as unknown as { context: () => Promise<unknown> }, 'context').mockResolvedValue({
status: 'success',
symbol: { name: 'rename_target', filePath: 'src/lib.rs', startLine: OCCURRENCE_LINES[0] },
incoming,
});
return backend;
}
function callRename(
backend: LocalBackend,
repoPath: string,
params: Record<string, unknown>,
): Promise<RenameResult> {
return (
backend as unknown as { rename: (r: unknown, p: unknown) => Promise<RenameResult> }
).rename({ repoPath }, { symbol_name: 'rename_target', new_name: 'renamed_fn', ...params });
}
const editsFor = (r: RenameResult, file: string) =>
r.changes.find((c) => c.file_path === file)?.edits ?? [];
describe('rename edit report is faithful to apply (#2605)', () => {
let tmpDir: string;
beforeEach(async () => {
execFileSyncMock.mockReturnValue(''); // default: no ripgrep hits
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-2605-'));
await fs.mkdir(path.join(tmpDir, 'src'));
await fs.writeFile(path.join(tmpDir, 'src', 'lib.rs'), RUST_SRC, 'utf-8');
});
afterEach(async () => {
vi.restoreAllMocks();
await fs.rm(tmpDir, { recursive: true, force: true });
});
it('previews every occurrence that apply will rewrite (dry_run)', async () => {
const result = await callRename(stubbedBackend(), tmpDir, { dry_run: true });
expect(result.applied).toBe(false);
expect(result.files_affected).toBe(1);
expect(result.total_edits).toBe(OCCURRENCE_LINES.length); // 4, not 1
// Concrete split, not just the sum: all occurrences are in the definition
// file, so they are graph-confidence and text_search is zero.
expect(result.graph_edits).toBe(OCCURRENCE_LINES.length);
expect(result.text_search_edits).toBe(0);
const edits = editsFor(result, 'src/lib.rs');
expect(edits.map((e) => e.line).sort((a, b) => a - b)).toEqual(OCCURRENCE_LINES);
expect(edits.every((e) => e.confidence === 'graph')).toBe(true);
// A dry run leaves the file untouched.
const onDisk = await fs.readFile(path.join(tmpDir, 'src', 'lib.rs'), 'utf-8');
expect(onDisk).toContain('rename_target');
});
it('reports exactly what it wrote (apply)', async () => {
const result = await callRename(stubbedBackend(), tmpDir, { dry_run: false });
expect(result.applied).toBe(true);
expect(result.total_edits).toBe(OCCURRENCE_LINES.length);
const onDisk = await fs.readFile(path.join(tmpDir, 'src', 'lib.rs'), 'utf-8');
const renamedCount = (onDisk.match(/\brenamed_fn\b/g) || []).length;
const stragglers = (onDisk.match(/\brename_target\b/g) || []).length;
expect(renamedCount).toBe(OCCURRENCE_LINES.length); // all 4 rewritten
expect(stragglers).toBe(0);
// The reported edit count equals the number of replacements that landed.
const reportedEdits = result.changes.reduce((n, c) => n + c.edits.length, 0);
expect(reportedEdits).toBe(renamedCount);
});
it('enumerates all occurrences across graph-ref and text-search files, keeping confidence per file', async () => {
// A graph-referencing file (not the definition) with MULTIPLE occurrences —
// the exact "one edit per file then break" bug's other original trigger.
const CALLER = 'use crate::rename_target;\nfn a() { rename_target(1); rename_target(2); }\n';
// A file discovered only by ripgrep — the text_search branch.
const NOTES = '// see rename_target for details\n';
await fs.writeFile(path.join(tmpDir, 'src', 'caller.rs'), CALLER, 'utf-8');
await fs.writeFile(path.join(tmpDir, 'src', 'notes.rs'), NOTES, 'utf-8');
// rg reports the definition file (already graph — exercises never-downgrade)
// and the text-only file.
execFileSyncMock.mockReturnValue('src/lib.rs\nsrc/notes.rs\n');
const backend = stubbedBackend({
...EMPTY_INCOMING,
calls: [{ filePath: 'src/caller.rs' }],
});
const result = await callRename(backend, tmpDir, { dry_run: true });
const callerOcc = occurrenceLines(CALLER).length; // 3
const notesOcc = occurrenceLines(NOTES).length; // 1
expect(result.files_affected).toBe(3);
expect(result.total_edits).toBe(OCCURRENCE_LINES.length + callerOcc + notesOcc);
// Split is concrete: definition + graph-ref file are graph; the rg-only file
// is text_search. A file reached by both graph and rg keeps graph (never
// downgraded).
expect(result.graph_edits).toBe(OCCURRENCE_LINES.length + callerOcc);
expect(result.text_search_edits).toBe(notesOcc);
expect(editsFor(result, 'src/lib.rs').every((e) => e.confidence === 'graph')).toBe(true);
expect(editsFor(result, 'src/caller.rs').map((e) => e.confidence)).toEqual(['graph', 'graph']);
expect(editsFor(result, 'src/notes.rs').map((e) => e.confidence)).toEqual(['text_search']);
});
it('reports only files that landed when a write fails mid-apply (#2605 partial)', async () => {
const CALLER = 'fn a() { rename_target(1); rename_target(2); }\n';
await fs.writeFile(path.join(tmpDir, 'src', 'caller.rs'), CALLER, 'utf-8');
const backend = stubbedBackend({ ...EMPTY_INCOMING, calls: [{ filePath: 'src/caller.rs' }] });
// caller.rs write throws; lib.rs succeeds.
vi.spyOn(fsPromises, 'writeFile').mockImplementation(
async (p: Parameters<typeof fsPromises.writeFile>[0]) => {
if (String(p).endsWith(`${path.sep}caller.rs`)) {
throw Object.assign(new Error('EACCES'), { code: 'EACCES' });
}
},
);
const result = await callRename(backend, tmpDir, { dry_run: false });
expect(result.status).toBe('partial');
expect(result.failed_files).toEqual(['src/caller.rs']);
// The failed file's edits are NOT reported as applied: totals describe only
// what reached disk (lib.rs), never the attempted caller.rs occurrences.
expect(result.files_affected).toBe(1);
expect(result.total_edits).toBe(OCCURRENCE_LINES.length);
expect(result.graph_edits).toBe(OCCURRENCE_LINES.length);
expect(result.changes.map((c) => c.file_path)).toEqual(['src/lib.rs']);
});
});