diff --git a/gitnexus/bench/mcp-tools-list/baselines.json b/gitnexus/bench/mcp-tools-list/baselines.json index 2f5b405a4..285b8613a 100644 --- a/gitnexus/bench/mcp-tools-list/baselines.json +++ b/gitnexus/bench/mcp-tools-list/baselines.json @@ -8,8 +8,8 @@ "list_repos": 200, "_shape_note": "THE FLOOR. Without these three, every ratio below is a ceiling over nothing. listTools_vs_listRepos_ratio only asserts something while the corpus still pays N parallel rev-list processes. Shrink it to three happy-path rows and both arms are cheap; the ratio still passes, asserting a property the corpus no longer has.", - "tools_listed": 19, - "_tools_note": "Exact GITNEXUS_TOOLS roster size returned by client.listTools(). 19 after read_file and grep were added as the MCP twins of GET /api/file and GET /api/grep (was 17). GITNEXUS_TOOLS.length and listTools must move together; a schema path that throws, filters, or returns [] still looks fast on the ratio arm.", + "tools_listed": 20, + "_tools_note": "Exact GITNEXUS_TOOLS roster size returned by client.listTools(). 20 after rename_preview was added for read-only graph-backed semantic rename planning (was 19 after read_file and grep, the MCP twins of GET /api/file and GET /api/grep). GITNEXUS_TOOLS.length and listTools must move together; a schema path that throws, filters, or returns [] still looks fast on the ratio arm.", "schema_read_only_requires_repo": true, "schema_mutating_requires_repo": true, diff --git a/gitnexus/bench/tool-accuracy/README.md b/gitnexus/bench/tool-accuracy/README.md index d91e11232..3b1fff34e 100644 --- a/gitnexus/bench/tool-accuracy/README.md +++ b/gitnexus/bench/tool-accuracy/README.md @@ -45,6 +45,15 @@ the cap/consumer contract without requiring 501 parser fixtures. It does not claim parser coverage for those synthetic records. Python checks read actual `context.outgoing.calls` symbol UIDs, preserving lexical/class ownership. +Rename's positive fixtures construct each `Writer` receiver directly, including +the imported class. A TypeScript parameter annotation alone cannot prove which +structurally compatible runtime object owns a method; unsafe parameter receivers +are covered by refusal regressions in the semantic rename unit tests. Fixture +line anchors are updated together; the three exact semantic answers and the +same-named method/function, comment and string controls remain required. The runner validates v2 +semantic provenance, exact UTF-16 spans and before/after source lines, then scores +the whole `after` line. Semantic rename does not use ripgrep or text-search edits. + ## Known gaps and repairs The initial source baseline is diff --git a/gitnexus/bench/tool-accuracy/expectations.ts b/gitnexus/bench/tool-accuracy/expectations.ts index c6dbe5248..1f28c85c9 100644 --- a/gitnexus/bench/tool-accuracy/expectations.ts +++ b/gitnexus/bench/tool-accuracy/expectations.ts @@ -6,14 +6,14 @@ */ export const FIXTURE_ANCHORS = { writerCloseDeclaration: { file: 'rename/writer.ts', line: 2, text: 'close(): void {}' }, - writerCloseCall: { file: 'rename/writer.ts', line: 8, text: 'writer.close();' }, - callerCloseCall: { file: 'rename/caller.ts', line: 3, text: 'writer.close();' }, + writerCloseCall: { file: 'rename/writer.ts', line: 9, text: 'writer.close();' }, + callerCloseCall: { file: 'rename/caller.ts', line: 4, text: 'writer.close();' }, writerComment: { file: 'rename/writer.ts', - line: 10, + line: 11, text: '// close is a comment, not a reference.', }, - writerString: { file: 'rename/writer.ts', line: 11, text: "export const title = 'close';" }, + writerString: { file: 'rename/writer.ts', line: 12, text: "export const title = 'close';" }, runGit: { file: 'src/security.ts', line: 4, diff --git a/gitnexus/bench/tool-accuracy/fixtures/rename/caller.ts b/gitnexus/bench/tool-accuracy/fixtures/rename/caller.ts index 321d3e926..18dc38018 100644 --- a/gitnexus/bench/tool-accuracy/fixtures/rename/caller.ts +++ b/gitnexus/bench/tool-accuracy/fixtures/rename/caller.ts @@ -1,4 +1,5 @@ import { Writer } from './writer'; -export function useImported(writer: Writer): void { +export function useImported(): void { + const writer = new Writer(); writer.close(); } diff --git a/gitnexus/bench/tool-accuracy/fixtures/rename/writer.ts b/gitnexus/bench/tool-accuracy/fixtures/rename/writer.ts index 66c80fc59..1f4913c3f 100644 --- a/gitnexus/bench/tool-accuracy/fixtures/rename/writer.ts +++ b/gitnexus/bench/tool-accuracy/fixtures/rename/writer.ts @@ -4,7 +4,8 @@ export class Writer { export class Other { close(): void {} } -export function useWriter(writer: Writer): void { +export function useWriter(): void { + const writer = new Writer(); writer.close(); } // close is a comment, not a reference. diff --git a/gitnexus/bench/tool-accuracy/known-gaps.json b/gitnexus/bench/tool-accuracy/known-gaps.json index fa0c72f60..e2371924c 100644 --- a/gitnexus/bench/tool-accuracy/known-gaps.json +++ b/gitnexus/bench/tool-accuracy/known-gaps.json @@ -2,24 +2,6 @@ "schemaVersion": 1, "baselineSourceSha": "dd096e690c06e5d54ab3595c63b83f15e04b6191", "gaps": [ - { - "id": "rename.comments-strings", - "issue": 3486, - "reason": "Whole-file word replacement rewrites comments and strings (#3486).", - "baselineActual": [ - "rename/writer.ts:10:// closeWriter is a comment, not a reference.", - "rename/writer.ts:11:export const title = 'closeWriter';" - ] - }, - { - "id": "rename.homonyms", - "issue": 3486, - "reason": "Whole-file word replacement rewrites unrelated same-named symbols (#3486).", - "baselineActual": [ - "rename/homonym.ts:1:export function closeWriter(): string {", - "rename/writer.ts:5:closeWriter(): void {}" - ] - }, { "id": "routes.helper-registration", "issue": 3487, diff --git a/gitnexus/bench/tool-accuracy/run.ts b/gitnexus/bench/tool-accuracy/run.ts index aebb4144f..ae9d0f034 100644 --- a/gitnexus/bench/tool-accuracy/run.ts +++ b/gitnexus/bench/tool-accuracy/run.ts @@ -82,27 +82,6 @@ async function fixtureDigest( return relative ? '' : digest.digest('hex'); } -// rename's text-search pass shells out to ripgrep and degrades silently when it is missing, -// which would change what the corpus measures. Fail closed before indexing instead. -const externalTools = Object.fromEntries( - ['rg'].map((tool) => { - try { - const version = execFileSync(tool, ['--version'], { - encoding: 'utf8', - timeout: 10_000, - stdio: ['ignore', 'pipe', 'ignore'], - }) - .split('\n')[0] - .trim(); - return [tool, version]; - } catch (error) { - throw new Error( - `Required external tool "${tool}" is unavailable (${error instanceof Error ? error.message : String(error)}); the tool-accuracy corpus is only valid when it is installed`, - ); - } - }), -); - const source = { sha: execFileSync('git', ['rev-parse', 'HEAD'], { cwd: CHECKOUT, encoding: 'utf8' }).trim(), dirty: !!execFileSync('git', ['status', '--porcelain', '--untracked-files=no'], { @@ -110,7 +89,6 @@ const source = { encoding: 'utf8', }).trim(), fixtureSha: await fixtureDigest(path.join(HERE, 'fixtures')), - externalTools, }; const manifest: KnownGapManifest = JSON.parse(await fs.readFile(knownGaps, 'utf8')); const temp = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-tool-accuracy-')); @@ -249,30 +227,84 @@ try { dry_run: true, }); need( - result.status === 'success' && result.applied === false && Array.isArray(result.changes), + result.result_version === 2 && + result.status === 'success' && + result.planning_status === 'ready' && + result.application_status === 'not_requested' && + result.applied === false && + result.old_name === 'close' && + result.new_name === 'closeWriter' && + result.text_search === 'not_used' && + result.coverage?.name === 'gitnexus-semantic' && + result.coverage.scope === 'indexed-repository' && + Array.isArray(result.changes), 'Malformed rename preview', ); const edits: Array<{ file: string; line: number; text: string }> = []; + const files = new Set(); for (const change of result.changes) { need( - typeof change.file_path === 'string' && Array.isArray(change.edits), + typeof change.file_path === 'string' && + change.file_path.length > 0 && + !files.has(change.file_path) && + Array.isArray(change.edits) && + change.edits.length > 0, 'Malformed rename file edits', ); + const absolute = path.resolve(repoPath, change.file_path); + const relative = path.relative(repoPath, absolute); + need( + relative && + relative !== '..' && + !relative.startsWith(`..${path.sep}`) && + !path.isAbsolute(relative), + 'Rename edit outside fixture repository', + ); + files.add(change.file_path); + const snapshot = await fs.readFile(absolute, 'utf8'); + let previousEnd = 0; for (const edit of change.edits) { need( - integer(edit.line, 1) && - typeof edit.old_text === 'string' && - typeof edit.new_text === 'string' && - ['graph', 'text_search'].includes(edit.confidence), + Number.isSafeInteger(edit.line) && + edit.line >= 1 && + Number.isSafeInteger(edit.start) && + edit.start >= previousEnd && + Number.isSafeInteger(edit.length) && + edit.length > 0 && + edit.start + edit.length <= snapshot.length && + edit.old_text === result.old_name && + edit.new_text === result.new_name && + edit.confidence === 'semantic' && + typeof edit.before === 'string' && + typeof edit.after === 'string', 'Malformed rename edit', ); - edits.push({ file: change.file_path, line: edit.line, text: edit.new_text }); + const lineStart = snapshot.lastIndexOf('\n', edit.start - 1) + 1; + const nextLine = snapshot.indexOf('\n', edit.start); + const lineEnd = nextLine < 0 ? snapshot.length : nextLine; + need( + snapshot.slice(edit.start, edit.start + edit.length) === edit.old_text && + edit.start + edit.length <= lineEnd && + edit.line === snapshot.slice(0, edit.start).split('\n').length && + edit.before === snapshot.slice(lineStart, lineEnd) && + edit.after === + snapshot.slice(lineStart, edit.start) + + edit.new_text + + snapshot.slice(edit.start + edit.length, lineEnd), + 'Inconsistent rename span or line context', + ); + previousEnd = edit.start + edit.length; + // v2 replacement text is an identifier; fixed answers retain whole source lines. + edits.push({ file: change.file_path, line: edit.line, text: edit.after.trim() }); } } need( integer(result.total_edits) && + integer(result.semantic_edits) && result.total_edits === edits.length && - result.graph_edits + result.text_search_edits === edits.length && + result.semantic_edits === edits.length && + result.graph_edits === 0 && + result.text_search_edits === 0 && result.files_affected === result.changes.length, 'Inconsistent rename totals', ); diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 155b2d26f..1ec98e459 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -6785,6 +6785,8 @@ export class LocalBackend { file_path: params.file_path, }); if (lookupResult.status === 'ambiguous' || lookupResult.error) return lookupResult; + const unavailable = await this.fullSourceUnavailable(repo); + if (unavailable) return unavailable; // Read the indexed file and declaration inventory, without context()'s // ranked/truncated incoming edges. Current sources rebuild semantic scopes; // a graph edge alone never authorizes a textual replacement. diff --git a/gitnexus/src/mcp/local/rename/graph-rename.ts b/gitnexus/src/mcp/local/rename/graph-rename.ts index 7d330923a..4bc281de9 100644 --- a/gitnexus/src/mcp/local/rename/graph-rename.ts +++ b/gitnexus/src/mcp/local/rename/graph-rename.ts @@ -29,6 +29,16 @@ import { import { lookupNameClaim } from '../../../core/ingestion/scope-resolution/scope/walkers.js'; import { definitionIdPosition } from '../../../core/ingestion/scope-resolution/utils/definition-id.js'; import { createParserForLanguage } from '../../../core/tree-sitter/parser-loader.js'; +import { + constructorForReceiver, + hasPythonMangledNames, + hasUnsupportedIdentifiers, + inDynamicWithScope, + isReceiverWrite, + jsxTagRole, + sameHoistedVarBinding, + unsupportedIdentifierSpelling, +} from './rename-safety.js'; import { RenameFailure, repositoryPath, @@ -91,7 +101,8 @@ export async function planGraphRename( if ( typeof options.new_name !== 'string' || !options.new_name || - options.new_name === symbol.name + options.new_name === symbol.name || + unsupportedIdentifierSpelling(options.new_name, language) ) { throw new RenameFailure('invalid_name', 'Choose a different, nonempty identifier.'); } @@ -162,6 +173,13 @@ export async function planGraphRename( if (!tree || tree.rootNode.hasError) { throw new RenameFailure('parse_error', `Cannot establish semantic coverage for ${filePath}.`); } + if (hasUnsupportedIdentifiers(tree.rootNode, language)) { + throw new RenameFailure( + 'unsupported_identifier', + `The provider cannot prove equivalent identifier spellings in ${filePath}.`, + 'unsupported', + ); + } const parsed = extractParsedFile(resolver.languageProvider, content, filePath, undefined, tree); if (!parsed) throw new RenameFailure( @@ -259,6 +277,22 @@ export async function planGraphRename( ); } const target = targetDefs[0]!; + if ( + language === SupportedLanguages.Python && + CLASS_KINDS.includes(target.type) && + files + .get(target.filePath)! + .namedOccurrences.some( + (token) => + target.nameRange && contains(target.nameRange, token) && hasPythonMangledNames(token), + ) + ) { + throw new RenameFailure( + 'unsupported_symbol_family', + 'Renaming a Python class with private-name or slot coupling requires a complete symbol-family plan.', + 'unsupported', + ); + } if ( CLASS_KINDS.includes(target.type) && (definitions.some( @@ -310,6 +344,10 @@ export async function planGraphRename( providers: { arityCompatibility: resolver.arityCompatibility }, ownedMembersByOwner: (owner, name) => lookupOwnedMembersByOwner(model, owner, name), }); + const isJsTs = + language === SupportedLanguages.TypeScript || language === SupportedLanguages.JavaScript; + const receiverWrites = new Map(); + const constructorProofs = new Map(); const boundMembers = (site: ReferenceSite): readonly SymbolDefinition[] | undefined => { if (!site.explicitReceiver) return undefined; const receiverClaim = lookupNameClaim(site.inScope, site.explicitReceiver.name, indexes, { @@ -317,6 +355,55 @@ export async function planGraphRename( purpose: 'value', }); const type = receiverClaim.typeBinding; + const receiverFile = receiverClaim.scope && files.get(receiverClaim.scope.filePath); + if (receiverFile && receiverClaim.scope) { + const bindingKey = `${receiverClaim.scope.id}:${site.explicitReceiver.name}:${JSON.stringify(type?.bindingRange)}`; + let hasWrites = receiverWrites.get(bindingKey); + if (hasWrites === undefined) { + const initializer = + isJsTs && + type && + constructorForReceiver(receiverFile.tree.rootNode, site.explicitReceiver.name, type); + hasWrites = receiverFile.tokens.some((token) => { + if (token.text !== site.explicitReceiver!.name || !isReceiverWrite(token)) return false; + // Only this exact constructor declaration is an initial value proof. + // Any other initialized declaration may reassign a hoisted var. + if (initializer && initializer.parent?.parent?.childForFieldName('name')?.id === token.id) + return false; + if (initializer && sameHoistedVarBinding(token, initializer)) return true; + // Python and similar grammars use assignment for the initial binding. + // JS/TS declarations have a distinct declarator; every assignment is + // an additional write even when it supplied the merged TypeRef. + if ( + !isJsTs && + type?.source === 'constructor-inferred' && + type.bindingRange && + contains(type.bindingRange, token) + ) + return false; + const scope = innermostScope(receiverFile.parsed, token); + const writeClaim = + scope && + lookupNameClaim(scope, token.text, indexes, { + position: { + startLine: token.startPosition.row + 1, + startCol: token.startPosition.column, + }, + purpose: 'value', + }); + // A constructor-inferred fact in a nested function can claim a name + // without declaring it. Only lexical claims prove an unrelated write. + return ( + !writeClaim || + !writeClaim.scope || + writeClaim.claims.length === 0 || + writeClaim.scope.id === receiverClaim.scope!.id + ); + }); + receiverWrites.set(bindingKey, hasWrites); + } + if (hasWrites) return []; + } if ( type?.source === 'decorator-unknown' || (type?.declaredSpelling && type.declaredSpelling !== type.rawName) @@ -336,6 +423,30 @@ export async function planGraphRename( ).values(), ]; if (classOwners.length !== 1) return undefined; + if (type && type.source !== 'self' && isJsTs) { + const proofKey = `${receiverClaim.scope?.id}:${site.explicitReceiver.name}:${JSON.stringify(type)}:${classOwners[0]!.nodeId}`; + let proven = constructorProofs.get(proofKey); + if (proven === undefined) { + const constructor = + receiverFile && + constructorForReceiver(receiverFile.tree.rootNode, site.explicitReceiver.name, type); + const constructorScope = constructor && innermostScope(receiverFile!.parsed, constructor); + const constructorOwners = + constructor && constructorScope && constructor.type === 'identifier' + ? lookupNameClaim(constructorScope, constructor.text, indexes, { + position: { + startLine: constructor.startPosition.row + 1, + startCol: constructor.startPosition.column, + }, + purpose: 'value', + }).bindings + : []; + const ids = new Set(constructorOwners.map((owner) => owner.def.nodeId)); + proven = ids.size === 1 && ids.has(classOwners[0]!.nodeId); + constructorProofs.set(proofKey, proven); + } + if (!proven) return []; + } return lookupOwnedMembersByOwner(model, classOwners[0]!.nodeId, site.name); }; const edits = new Map(); @@ -417,6 +528,13 @@ export async function planGraphRename( 'A reference points outside the indexed source set.', ); const token = tokenAtRange(file, site.nameRange); + if (isJsTs && inDynamicWithScope(token)) { + throw new RenameFailure( + 'unsupported_occurrence', + `A dynamic with environment prevents lexical rename proof in ${file.path}.`, + 'unsupported', + ); + } if (accounted.has(key(file.path, token.startIndex))) continue; const claim = site.explicitReceiver === undefined @@ -550,7 +668,8 @@ export async function planGraphRename( !replacement || replacement.text !== options.new_name || replacement.type !== original.type || - replacement.parent?.type !== original.parent?.type + replacement.parent?.type !== original.parent?.type || + jsxTagRole(replacement) !== jsxTagRole(original) ) { throw new RenameFailure( 'invalid_name', @@ -667,13 +786,7 @@ function isUnrelatedLabel(token: Parser.SyntaxNode): boolean { if (parent.type === 'keyword_argument' && parent.childForFieldName('name')?.id === token.id) return true; if (parent.type === 'jsx_attribute' && parent.firstNamedChild?.id === token.id) return true; - return ( - ['jsx_opening_element', 'jsx_closing_element', 'jsx_self_closing_element'].includes( - parent.type, - ) && - parent.childForFieldName('name')?.id === token.id && - /^[a-z]/.test(token.text) - ); + return jsxTagRole(token) === 'intrinsic'; } function isParameterBinder(token: Parser.SyntaxNode): boolean { diff --git a/gitnexus/src/mcp/local/rename/rename-plan.ts b/gitnexus/src/mcp/local/rename/rename-plan.ts index 68ce6737e..3dafc14dc 100644 --- a/gitnexus/src/mcp/local/rename/rename-plan.ts +++ b/gitnexus/src/mcp/local/rename/rename-plan.ts @@ -146,8 +146,8 @@ function blocked(symbol: RenameSymbol, newName: string, error: unknown): RenameR }; } -async function validateSnapshots(plan: RenamePlan): Promise { - for (const [file, snapshot] of plan.snapshots) { +async function validateSnapshots(snapshots: ReadonlyMap): Promise { + for (const [file, snapshot] of snapshots) { if (!(await fs.readFile(file)).equals(Buffer.from(snapshot, 'utf8'))) { throw new RenameFailure( 'source_changed', @@ -244,13 +244,14 @@ export async function executeRenamePlan( }), }); } - await validateSnapshots(plan); + await validateSnapshots(plan.snapshots); } catch (error) { return { ...blocked(plan.symbol, plan.new_name, error), coverage: plan.coverage }; } if (dryRun) return { ...result(plan.symbol, plan.new_name, changes), coverage: plan.coverage }; const landed: FileChange[] = []; + const expectedSnapshots = new Map(plan.snapshots); for (const change of changes) { const file = files.get(change.file_path)!; let temporaryDirectory: string | undefined; @@ -275,16 +276,17 @@ export async function executeRenamePlan( const replacement = path.join(temporaryDirectory, 'replacement'); await fs.writeFile(replacement, file.output, 'utf8'); await fs.chmod(replacement, mode); - if ( - (await checkedRealPath(repoPath, change.file_path)) !== file.real || - !(await fs.readFile(file.absolute)).equals(Buffer.from(file.snapshot, 'utf8')) - ) { + if ((await checkedRealPath(repoPath, change.file_path)) !== file.real) { throw new RenameFailure( 'source_changed', `Source changed before replacing ${change.file_path}.`, ); } + // Unedited sources also contribute semantic evidence. Already installed + // files must match their landed outputs rather than their old snapshots. + await validateSnapshots(expectedSnapshots); await fs.rename(replacement, file.real); + expectedSnapshots.set(file.absolute, file.output); landed.push(change); } catch (error) { return { diff --git a/gitnexus/src/mcp/local/rename/rename-safety.ts b/gitnexus/src/mcp/local/rename/rename-safety.ts new file mode 100644 index 000000000..8a367f692 --- /dev/null +++ b/gitnexus/src/mcp/local/rename/rename-safety.ts @@ -0,0 +1,204 @@ +import type Parser from 'tree-sitter'; +import { SupportedLanguages, type Range, type TypeRef } from 'gitnexus-shared'; + +/** Providers currently expose written names, not canonical identifier identities. */ +export function unsupportedIdentifierSpelling(name: string, language: SupportedLanguages): boolean { + return ( + name.includes('\\') || + name.startsWith('@') || + name.startsWith('r#') || + name.startsWith('`') || + (language === SupportedLanguages.Python && name.normalize('NFKC') !== name) + ); +} + +export function hasUnsupportedIdentifiers( + root: Parser.SyntaxNode, + language: SupportedLanguages, +): boolean { + const pending = [root]; + while (pending.length) { + const node = pending.pop()!; + if (node.type.includes('comment')) continue; + // Escaped identifiers can have named escape children and therefore never + // enter the planner's leaf-token audit. Inspect the identifier itself. + if (node.type.includes('identifier') && unsupportedIdentifierSpelling(node.text, language)) + return true; + pending.push(...node.namedChildren); + } + return false; +} + +export function inDynamicWithScope(token: Parser.SyntaxNode): boolean { + for (let node = token.parent; node; node = node.parent) { + if (node.type === 'with_statement') return true; + } + return false; +} + +export function jsxTagRole(token: Parser.SyntaxNode): 'intrinsic' | 'component' | undefined { + const parent = token.parent; + if ( + parent && + ['jsx_opening_element', 'jsx_closing_element', 'jsx_self_closing_element'].includes( + parent.type, + ) && + parent.childForFieldName('name')?.id === token.id + ) + return /^[a-z]/.test(token.text) ? 'intrinsic' : 'component'; + return undefined; +} + +export function hasPythonMangledNames(className: Parser.SyntaxNode): boolean { + const declaration = className.parent; + if (declaration?.type !== 'class_definition') return false; + const pending = [...declaration.namedChildren]; + const isPrivate = (name: string) => name.startsWith('__') && !name.endsWith('__'); + while (pending.length) { + const node = pending.pop()!; + // A nested class body establishes its own mangling context. Its name and + // base expressions are still evaluated in the containing class. + if (node.type === 'class_definition') { + if (isPrivate(node.childForFieldName('name')?.text ?? '')) return true; + const body = node.childForFieldName('body'); + pending.push(...node.namedChildren.filter((child) => child.id !== body?.id)); + continue; + } + if (node.type === 'identifier' && isPrivate(node.text)) return true; + if ( + node.type === 'identifier' && + node.text === '__slots__' && + bindsInPythonClassBody(node, declaration) + ) + return true; + pending.push(...node.namedChildren); + } + return false; +} + +function bindsInPythonClassBody(token: Parser.SyntaxNode, declaration: Parser.SyntaxNode): boolean { + for (let node = token.parent; node && node.id !== declaration.id; node = node.parent) { + if (node.type !== 'function_definition' && node.type !== 'lambda') continue; + // Function bodies and parameter binders do not populate the class dictionary; + // default values and annotations still evaluate in the enclosing context. + const body = node.childForFieldName('body'); + if (body && within(token, body)) return false; + const parameters = node.childForFieldName('parameters'); + if (parameters && within(token, parameters)) { + let parameter = token; + while (parameter.parent && parameter.parent.id !== parameters.id) + parameter = parameter.parent; + const value = parameter.childForFieldName('value'); + const annotation = parameter.childForFieldName('type'); + if (!(value && within(token, value)) && !(annotation && within(token, annotation))) + return false; + } + } + return true; +} + +function within(node: Parser.SyntaxNode, container: Parser.SyntaxNode): boolean { + return node.startIndex >= container.startIndex && node.endIndex <= container.endIndex; +} + +/** Syntactic writes only: their reaching value is deliberately not inferred. */ +export function isReceiverWrite(token: Parser.SyntaxNode): boolean { + for (let node = token.parent; node; node = node.parent) { + // A property write does not rebind the object used to reach that property. + if (['member_expression', 'attribute', 'field_expression'].includes(node.type)) return false; + if (node.type === 'variable_declarator') { + const name = node.childForFieldName('name'); + return !!name && within(token, name) && node.childForFieldName('value') !== null; + } + if (node.type.includes('assignment') || node.type === 'for_in_statement') { + const left = node.childForFieldName('left'); + if (left && within(token, left)) return true; + } + if (node.type === 'update_expression') return true; + if (node.type.includes('statement') || node.type.includes('declaration')) return false; + } + return false; +} + +/** Block-local provider claims cannot separate redeclarations of one hoisted var. */ +export function sameHoistedVarBinding( + token: Parser.SyntaxNode, + initialConstructor: Parser.SyntaxNode, +): boolean { + const writeScope = hoistedVarScope(token.parent); + return ( + writeScope !== undefined && + writeScope.id === hoistedVarScope(initialConstructor.parent?.parent)?.id + ); +} + +function hoistedVarScope( + declaration: Parser.SyntaxNode | null | undefined, +): Parser.SyntaxNode | undefined { + while (declaration && declaration.type !== 'variable_declarator') { + if (declaration.type.includes('statement') || declaration.type.includes('declaration')) + return undefined; + declaration = declaration.parent; + } + if ( + declaration?.type !== 'variable_declarator' || + !declaration.parent?.children.some((child) => child.type === 'var') + ) + return undefined; + for (let node: Parser.SyntaxNode | null = declaration.parent; node; node = node.parent) { + if ( + [ + 'program', + 'function_declaration', + 'function_expression', + 'arrow_function', + 'generator_function_declaration', + 'generator_function', + 'method_definition', + 'class_static_block', + ].includes(node.type) + ) + return node; + } + return undefined; +} + +/** TS structural annotations and merged type summaries are not receiver identity. */ +export function constructorForReceiver( + root: Parser.SyntaxNode, + name: string, + type: TypeRef, +): Parser.SyntaxNode | undefined { + if (!type.bindingRange) return undefined; + const range = type.bindingRange; + const pending = [root]; + while (pending.length) { + const node = pending.pop()!; + const start = node.startPosition; + const end = node.endPosition; + if ( + start.row + 1 > range.startLine || + (start.row + 1 === range.startLine && start.column > range.startCol) || + end.row + 1 < range.endLine || + (end.row + 1 === range.endLine && end.column < range.endCol) + ) + continue; + if (node.type === 'variable_declarator' && sameRange(node, range)) { + const value = node.childForFieldName('value'); + if (node.childForFieldName('name')?.text === name && value?.type === 'new_expression') + return value.childForFieldName('constructor') ?? undefined; + return undefined; + } + pending.push(...node.namedChildren); + } + return undefined; +} + +function sameRange(node: Parser.SyntaxNode, range: Range): boolean { + return ( + node.startPosition.row + 1 === range.startLine && + node.startPosition.column === range.startCol && + node.endPosition.row + 1 === range.endLine && + node.endPosition.column === range.endCol + ); +} diff --git a/gitnexus/test/unit/graph-rename.test.ts b/gitnexus/test/unit/graph-rename.test.ts index d3cecb27c..b3b7e2cd6 100644 --- a/gitnexus/test/unit/graph-rename.test.ts +++ b/gitnexus/test/unit/graph-rename.test.ts @@ -41,10 +41,11 @@ describe('graph and SemanticModel rename', () => { symbol: RenameSymbol, nodes: GraphNode[], code: string, + newName = 'Renamed', ) { for (const dry_run of [true, false]) { - const result = await renameSymbol(root, symbol, { new_name: 'Renamed', dry_run }, nodes); - expect(result).toMatchObject({ + const result = await renameSymbol(root, symbol, { new_name: newName, dry_run }, nodes); + expect(result, JSON.stringify(result)).toMatchObject({ status: 'error', application_status: 'not_started', applied: false, @@ -58,6 +59,540 @@ describe('graph and SemanticModel rename', () => { } } + it.each([ + [ + 'escaped JS reference', + 'model.js', + 'function target() {}\nt\\u0061rget();\n', + 'target', + 'Function', + ], + [ + 'escaped TS reference', + 'model.ts', + 'function target() {}\ntar\\u{67}et();\n', + 'target', + 'Function', + ], + [ + 'escaped shadow binder', + 'model.ts', + 'function target() {}\nfunction other(t\\u0061rget) { target(); }\n', + 'target', + 'Function', + ], + [ + 'escaped replacement collision', + 'model.ts', + 'function target() {}\nconst Re\\u006eamed = 1;\ntarget();\n', + 'target', + 'Function', + ], + ['Python normalized reference', 'model.py', 'def K(): pass\nK()\n', 'K', 'Function'], + [ + 'Python normalized shadow binder', + 'model.py', + 'def K(): pass\ndef other(K):\n K()\n', + 'K', + 'Function', + ], + [ + 'C# verbatim type reference', + 'model.cs', + 'class Target {}\nclass Use { @Target value; }\n', + 'Target', + 'Class', + ], + ] as const)( + 'blocks preview and apply for an unsupported %s', + async (_name, file, source, name, kind) => { + const nodes = await fixture(file, source); + nodes[1] = { + id: `${kind}:${file}:${name}`, + label: kind, + properties: { name, filePath: file, startLine: 0, endLine: 0 }, + }; + await expectBlockedPreviewAndApply( + file, + source, + { + uid: nodes[1].id, + name, + kind, + filePath: file, + startLine: 1, + }, + nodes, + 'unsupported_identifier', + ); + }, + ); + + it.each([''])( + 'blocks JSX component-to-intrinsic renames in preview and apply: %s', + async (element) => { + const file = 'model.tsx'; + const source = `function Target() { return null; }\nconst element = ${element};\n`; + const nodes = await fixture(file, source); + nodes[1] = { + id: `Function:${file}:Target`, + label: 'Function', + properties: { name: 'Target', filePath: file, startLine: 0, endLine: 0 }, + }; + await expectBlockedPreviewAndApply( + file, + source, + { + uid: nodes[1].id, + name: 'Target', + kind: 'Function', + filePath: file, + startLine: 1, + }, + nodes, + 'invalid_name', + 'widget', + ); + }, + ); + + it('preserves JSX component roles in a successful preview and apply', async () => { + const file = 'model.tsx'; + const source = 'function Target() { return null; }\nconst element = ;\n'; + const nodes = await fixture(file, source); + nodes[1] = { + id: `Function:${file}:Target`, + label: 'Function', + properties: { name: 'Target', filePath: file, startLine: 0, endLine: 0 }, + }; + const symbol = { + uid: nodes[1].id, + name: 'Target', + kind: 'Function', + filePath: file, + startLine: 1, + }; + const preview = await renameSymbol(root, symbol, { new_name: 'Widget', dry_run: true }, nodes); + expect(preview, JSON.stringify(preview)).toMatchObject({ status: 'success', total_edits: 2 }); + expect(await fs.readFile(path.join(root, file), 'utf8')).toBe(source); + const applied = await renameSymbol(root, symbol, { new_name: 'Widget', dry_run: false }, nodes); + expect(applied).toMatchObject({ status: 'success', applied: true, changes: preview.changes }); + expect(await fs.readFile(path.join(root, file), 'utf8')).toBe( + 'function Widget() { return null; }\nconst element = ;\n', + ); + }); + + it.each([ + ['private field', ' __secret = 42\n', 'w = Writer()\nprint(w._Writer__secret)\n'], + [ + 'private method', + ' def __secret(self): return 42\n', + 'w = Writer()\nprint(w._Writer__secret())\n', + ], + [ + 'private local in a method', + ' def read(self):\n __secret = 42\n return __secret\n', + 'w = Writer()\n', + ], + [ + 'private tuple slot', + ' __slots__ = ("__secret",)\n', + 'w = Writer()\nw._Writer__secret = 42\nprint(w._Writer__secret)\n', + ], + ['private string slot', ' __slots__ = "__secret"\n', 'w = Writer()\n'], + ['unknown slot expression', ' __slots__ = make_slots()\n', 'w = Writer()\n'], + [ + 'loop-bound private slots', + ' for __slots__ in [("__secret",)]: pass\n', + 'w = Writer()\nw._Writer__secret = 42\nprint(w._Writer__secret)\n', + ], + [ + 'named-expression private slots', + ' (__slots__ := ("__secret",))\n', + 'w = Writer()\nw._Writer__secret = 42\nprint(w._Writer__secret)\n', + ], + ])('blocks Python class renames with a mangled %s', async (_name, body, use) => { + const file = 'model.py'; + const source = `class Writer:\n${body}${use}`; + const uid = `Class:${file}:Writer`; + const nodes = await fixture(file, source, [ + { + id: uid, + label: 'Class', + properties: { name: 'Writer', filePath: file, startLine: 0 }, + }, + ]); + await expectBlockedPreviewAndApply( + file, + source, + { + uid, + name: 'Writer', + kind: 'Class', + filePath: file, + startLine: 1, + }, + nodes, + 'unsupported_symbol_family', + ); + }); + + it.each([ + ['shadow property', 'const object = { target: () => "object" };\nwith (object) target();\n'], + ['unknown object', 'with (object) target();\n'], + ['deferred closure', 'with (object) { function invoke() { return target(); } }\n'], + ])('blocks dynamic with bindings in preview and apply: %s', async (_name, use) => { + const file = 'model.js'; + const source = 'function target() { return "global"; }\n' + use; + const nodes = await fixture(file, source); + nodes[1]!.properties.startLine = 0; + await expectBlockedPreviewAndApply( + file, + source, + { + uid: nodes[1]!.id, + name: 'target', + kind: 'Function', + filePath: file, + startLine: 1, + }, + nodes, + 'unsupported_occurrence', + ); + }); + + it.each([ + [ + 'calls before and after a write', + 'let writer = new Writer();\nwriter.close();\nwriter = new Other();\nwriter.close();\n', + ], + [ + 'write before the call', + 'let writer = new Writer();\nwriter = new Other();\nwriter.close();\n', + ], + [ + 'write after the call', + 'let writer = new Writer();\nwriter.close();\nwriter = new Other();\n', + ], + [ + 'conditional write', + 'let writer = new Writer();\nif (false) { writer = new Other(); }\nwriter.close();\n', + ], + ['unknown write', 'let writer = new Writer();\nwriter = obtain();\nwriter.close();\n'], + [ + 'closure write', + 'let writer = new Writer();\nfunction change() { writer = new Other(); }\nwriter.close();\n', + ], + [ + 'destructuring write', + 'let writer = new Writer();\n[writer] = [new Other()];\nwriter.close();\n', + ], + ['structural initializer', 'const writer: Writer = new Other();\nwriter.close();\n'], + [ + 'structural parameter', + 'function use(writer: Writer) { writer.close(); }\nuse(new Other());\n', + ], + [ + 'conditional var redeclaration to another class', + 'var writer = new Writer();\nif (false) { var writer = new Other(); }\nwriter.close();\n', + 'model.js', + ], + [ + 'conditional var redeclaration to the target class', + 'var writer = new Other();\nif (false) { var writer = new Writer(); }\nwriter.close();\n', + 'model.js', + ], + [ + 'conditional var before the final declaration', + 'if (false) { var writer = new Other(); }\nvar writer = new Writer();\nwriter.close();\n', + 'model.js', + ], + [ + 'conditional target var before the final declaration', + 'if (false) { var writer = new Writer(); }\nvar writer = new Other();\nwriter.close();\n', + 'model.js', + ], + [ + 'function-hoisted conditional var redeclaration', + 'function use() {\nvar writer = new Writer();\nif (false) { var writer = new Other(); }\nwriter.close();\n}\nuse();\n', + 'model.js', + ], + ])( + 'blocks unproven receiver ownership in preview and apply: %s', + async (_name, use, file = 'model.ts') => { + const source = 'class Writer {\n close() {}\n}\nclass Other {\n close() {}\n}\n' + use; + const uid = `Method:${file}:Writer.close#0`; + const nodes = await fixture(file, source, [ + { + id: `Class:${file}:Writer`, + label: 'Class', + properties: { name: 'Writer', filePath: file, startLine: 0, endLine: 2 }, + }, + { + id: `Class:${file}:Other`, + label: 'Class', + properties: { name: 'Other', filePath: file, startLine: 3, endLine: 5 }, + }, + { + id: uid, + label: 'Method', + properties: { name: 'close', filePath: file, startLine: 1, endLine: 1 }, + }, + { + id: `Method:${file}:Other.close#0`, + label: 'Method', + properties: { name: 'close', filePath: file, startLine: 4, endLine: 4 }, + }, + ]); + await expectBlockedPreviewAndApply( + file, + source, + { + uid, + name: 'close', + kind: 'Method', + filePath: file, + startLine: 2, + }, + nodes, + 'ambiguous_reference', + ); + }, + ); + + it.each([ + ['constant receiver', 'const writer = new Writer();\nwriter.close();\n'], + ['unwritten mutable receiver', 'let writer = new Writer();\nwriter.close();\n'], + ['stable var receiver', 'var writer = new Writer();\nwriter.close();\n'], + [ + 'block let shadow', + 'const writer = new Writer();\nif (false) { let writer = obtain(); }\nwriter.close();\n', + ], + [ + 'block const shadow', + 'const writer = new Writer();\nif (false) { const writer = obtain(); }\nwriter.close();\n', + ], + [ + 'separate function var', + 'var writer = new Writer();\nfunction other() { var writer = obtain(); }\nwriter.close();\n', + ], + ['matching annotated initializer', 'const writer: Writer = new Writer();\nwriter.close();\n'], + [ + 'unrelated shadow write', + 'const writer = new Writer();\nfunction change(writer) { writer = obtain(); }\nwriter.close();\n', + ], + ])('preserves constructor-proven member preview and apply: %s', async (_name, use) => { + const file = 'model.ts'; + const source = 'class Writer {\n close() {}\n}\n' + use; + const uid = `Method:${file}:Writer.close#0`; + const nodes = await fixture(file, source, [ + { + id: `Class:${file}:Writer`, + label: 'Class', + properties: { name: 'Writer', filePath: file, startLine: 0, endLine: 2 }, + }, + { + id: uid, + label: 'Method', + properties: { name: 'close', filePath: file, startLine: 1, endLine: 1 }, + }, + ]); + const symbol = { uid, name: 'close', kind: 'Method', filePath: file, startLine: 2 }; + const preview = await renameSymbol(root, symbol, { new_name: 'finish', dry_run: true }, nodes); + expect(preview, JSON.stringify(preview)).toMatchObject({ status: 'success', total_edits: 2 }); + expect(preview.changes[0]!.edits.map((edit) => edit.start)).toEqual([ + source.indexOf('close'), + source.lastIndexOf('close'), + ]); + expect(await fs.readFile(path.join(root, file), 'utf8')).toBe(source); + const applied = await renameSymbol(root, symbol, { new_name: 'finish', dry_run: false }, nodes); + expect(applied).toMatchObject({ status: 'success', applied: true, changes: preview.changes }); + expect(await fs.readFile(path.join(root, file), 'utf8')).toBe( + source.replaceAll('close', 'finish'), + ); + }); + + it('preserves provider-proven this receiver ownership', async () => { + const file = 'model.ts'; + const source = 'class Writer {\n close() {}\n use() { this.close(); }\n}\n'; + const uid = `Method:${file}:Writer.close#0`; + const nodes = await fixture(file, source, [ + { + id: `Class:${file}:Writer`, + label: 'Class', + properties: { name: 'Writer', filePath: file, startLine: 0, endLine: 3 }, + }, + { + id: uid, + label: 'Method', + properties: { name: 'close', filePath: file, startLine: 1, endLine: 1 }, + }, + ]); + const plan = await planGraphRename( + root, + { + uid, + name: 'close', + kind: 'Method', + filePath: file, + startLine: 2, + }, + { new_name: 'finish' }, + nodes, + ); + expect(plan.edits.map((edit) => edit.start)).toEqual([ + source.indexOf('close'), + source.lastIndexOf('close'), + ]); + }); + + it('preserves Python lexical with scopes', async () => { + const file = 'model.py'; + const source = 'def target(): pass\nwith open("file") as stream:\n target()\n'; + const nodes = await fixture(file, source); + nodes[1]!.properties.startLine = 0; + const symbol = { + uid: nodes[1]!.id, + name: 'target', + kind: 'Function', + filePath: file, + startLine: 1, + }; + const preview = await renameSymbol(root, symbol, { new_name: 'renamed', dry_run: true }, nodes); + expect(preview, JSON.stringify(preview)).toMatchObject({ status: 'success', total_edits: 2 }); + expect(await fs.readFile(path.join(root, file), 'utf8')).toBe(source); + const applied = await renameSymbol( + root, + symbol, + { new_name: 'renamed', dry_run: false }, + nodes, + ); + expect(applied).toMatchObject({ status: 'success', applied: true, changes: preview.changes }); + expect(await fs.readFile(path.join(root, file), 'utf8')).toBe( + 'def renamed(): pass\nwith open("file") as stream:\n renamed()\n', + ); + }); + + it('preserves Python member renames for a single constructor assignment', async () => { + const file = 'model.py'; + const source = 'class Writer:\n def close(self): pass\nwriter = Writer()\nwriter.close()\n'; + const uid = `Method:${file}:Writer.close#0`; + const nodes = await fixture(file, source, [ + { + id: `Class:${file}:Writer`, + label: 'Class', + properties: { name: 'Writer', filePath: file, startLine: 0, endLine: 1 }, + }, + { + id: uid, + label: 'Method', + properties: { name: 'close', filePath: file, startLine: 1, endLine: 1 }, + }, + ]); + const plan = await planGraphRename( + root, + { + uid, + name: 'close', + kind: 'Method', + filePath: file, + startLine: 2, + }, + { new_name: 'finish' }, + nodes, + ); + expect(plan.edits.map((edit) => edit.start)).toEqual([ + source.indexOf('close'), + source.lastIndexOf('close'), + ]); + }); + + it('preserves Python class renames with ordinary dunder names', async () => { + const file = 'model.py'; + const source = 'class Writer:\n def __init__(self): pass\nwriter = Writer()\n'; + const uid = `Class:${file}:Writer`; + const nodes = await fixture(file, source, [ + { + id: uid, + label: 'Class', + properties: { name: 'Writer', filePath: file, startLine: 0, endLine: 1 }, + }, + ]); + const plan = await planGraphRename( + root, + { + uid, + name: 'Writer', + kind: 'Class', + filePath: file, + startLine: 1, + }, + { new_name: 'Renamed' }, + nodes, + ); + expect(plan.edits.map((edit) => edit.start)).toEqual([ + source.indexOf('Writer'), + source.lastIndexOf('Writer'), + ]); + }); + + it.each([ + ['nested class slots', ' class Nested:\n __slots__ = ("__secret",)\n'], + ['ordinary string', ' label = "__slots__ __secret"\n'], + ['method-local slots name', ' def use(self):\n __slots__ = ("__secret",)\n'], + ['method parameter slots name', ' def use(self, __slots__): pass\n'], + ['method default parameter slots name', ' def use(self, __slots__ = ("__secret",)): pass\n'], + ])('preserves Python class preview and apply with an unrelated %s', async (_name, body) => { + const file = 'model.py'; + const source = `class Writer:\n${body}writer = Writer()\n`; + const uid = `Class:${file}:Writer`; + const nodes = await fixture(file, source, [ + { + id: uid, + label: 'Class', + properties: { name: 'Writer', filePath: file, startLine: 0 }, + }, + ]); + const symbol = { uid, name: 'Writer', kind: 'Class', filePath: file, startLine: 1 }; + const preview = await renameSymbol(root, symbol, { new_name: 'Renamed', dry_run: true }, nodes); + expect(preview, JSON.stringify(preview)).toMatchObject({ status: 'success', total_edits: 2 }); + expect(await fs.readFile(path.join(root, file), 'utf8')).toBe(source); + const applied = await renameSymbol( + root, + symbol, + { new_name: 'Renamed', dry_run: false }, + nodes, + ); + expect(applied).toMatchObject({ status: 'success', applied: true, changes: preview.changes }); + expect(await fs.readFile(path.join(root, file), 'utf8')).toBe( + `class Renamed:\n${body}writer = Renamed()\n`, + ); + }); + + it('allows escaped identifier text in strings and comments', async () => { + const file = 'model.ts'; + const source = + 'function target() {}\nconst text = "t\\u0061rget";\n// t\\u0061rget\ntarget();\n'; + const nodes = await fixture(file, source); + nodes[1]!.properties.startLine = 0; + const plan = await planGraphRename( + root, + { + uid: nodes[1]!.id, + name: 'target', + kind: 'Function', + filePath: file, + startLine: 1, + }, + { new_name: 'renamed' }, + nodes, + ); + expect(plan.edits.map((edit) => edit.start)).toEqual([ + source.indexOf('target'), + source.lastIndexOf('target'), + ]); + }); + it('blocks PHP preview and apply until case-insensitive names have provider coverage', async () => { const file = 'model.php'; const source = ' { ]); }); - it('resolves a typed member through finalized cross-file import bindings', async () => { + it('resolves a constructed member through finalized cross-file import bindings', async () => { const file = 'model.ts'; const nodes = await fixture(file, 'export class Writer {\n close() {}\n}\n', [ { @@ -371,7 +906,7 @@ describe('graph and SemanticModel rename', () => { }, ]); const consumer = - 'import { Writer } from "./model.js";\nfunction use(writer: Writer) { writer.close(); }\n'; + 'import { Writer } from "./model.js";\nconst writer = new Writer();\nwriter.close();\n'; await fs.writeFile(path.join(root, 'consumer.ts'), consumer); nodes.push({ id: 'File:consumer.ts', diff --git a/gitnexus/test/unit/rename-edit-report.test.ts b/gitnexus/test/unit/rename-edit-report.test.ts index 46ea03373..3609c9251 100644 --- a/gitnexus/test/unit/rename-edit-report.test.ts +++ b/gitnexus/test/unit/rename-edit-report.test.ts @@ -39,7 +39,9 @@ const SOURCE = `export class Writer { export class Other { close() {} } -export function run(writer: Writer, other: Other) { +export function run() { + const writer = new Writer(); + const other = new Other(); const hooks = { close: () => writer.close() }; writer.close(); other.close(); // close should remain in this comment. const title = 'close'; @@ -47,7 +49,10 @@ export function run(writer: Writer, other: Other) { } `; const CALLER = `import { Writer } from './writer.js'; -export function caller(writer: Writer) { writer.close(); } +export function caller() { + const writer = new Writer(); + writer.close(); +} `; function stubbedBackend(symbol: Record = {}) { @@ -71,7 +76,7 @@ function stubbedBackend(symbol: Record = {}) { const callRename = (backend: LocalBackend, repoPath: string, params = {}) => (backend as any).rename( - { repoPath }, + { repoPath, lbugPath: path.join(repoPath, '.gitnexus', 'lbug') }, { symbol_name: 'close', new_name: 'closeWriter', @@ -125,17 +130,22 @@ describe('semantic rename reports exact applied occurrences (#3486, #2605)', () name: 'run', filePath: 'src/writer.ts', startLine: 6, - endLine: 11, + endLine: 13, }, { id: 'Function:src/caller.ts:caller', name: 'caller', filePath: 'src/caller.ts', startLine: 1, - endLine: 1, + endLine: 4, }, ); tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-3486-backend-')); + await fs.mkdir(path.join(tmpDir, '.gitnexus')); + await fs.writeFile( + path.join(tmpDir, '.gitnexus', 'gitnexus.json'), + JSON.stringify({ contentRetention: 'full', indexedAt: '2024-06-01T12:00:00Z' }), + ); await fs.mkdir(path.join(tmpDir, 'src')); await fs.writeFile(path.join(tmpDir, 'src/writer.ts'), SOURCE); await fs.writeFile(path.join(tmpDir, 'src/caller.ts'), CALLER); diff --git a/gitnexus/test/unit/rename-plan.test.ts b/gitnexus/test/unit/rename-plan.test.ts index f8802fd9d..acc2e1730 100644 --- a/gitnexus/test/unit/rename-plan.test.ts +++ b/gitnexus/test/unit/rename-plan.test.ts @@ -122,6 +122,49 @@ describe('exact rename plan application', () => { expect(await fs.readFile(path.join(root, 'a.ts'), 'utf8')).toBe('close()'); }); + it('accepts landed outputs while applying an ordinary multi-file plan', async () => { + const p = await plan({ 'a.ts': 'close()', 'b.ts': 'close()' }); + expect(await executeRenamePlan(root, p, false)).toMatchObject({ + status: 'success', + applied: true, + application_status: 'applied', + total_edits: 2, + files_affected: 2, + }); + expect(await fs.readFile(path.join(root, 'a.ts'), 'utf8')).toBe('finish()'); + expect(await fs.readFile(path.join(root, 'b.ts'), 'utf8')).toBe('finish()'); + expect((await fs.readdir(root)).sort()).toEqual(['a.ts', 'b.ts']); + }); + + it.each([0, 1])( + 'blocks installation when an unedited snapshot changes while staging file %i', + async (changeAt) => { + const p = await plan({ 'a.ts': 'close()', 'b.ts': 'close()', 'c.ts': '// no references' }); + p.edits.pop(); + const original = fs.writeFile.bind(fs); + let calls = 0; + vi.spyOn(fs, 'writeFile').mockImplementation(async (...args) => { + await original(...args); + if (calls++ === changeAt) await original(path.join(root, 'c.ts'), 'close();'); + }); + expect(await executeRenamePlan(root, p, false)).toMatchObject({ + status: 'partial', + applied: changeAt > 0, + application_status: changeAt ? 'partial' : 'failed', + code: 'source_changed', + total_edits: changeAt, + files_affected: changeAt, + failed_files: changeAt ? ['b.ts'] : ['a.ts', 'b.ts'], + }); + expect(await fs.readFile(path.join(root, 'a.ts'), 'utf8')).toBe( + changeAt ? 'finish()' : 'close()', + ); + expect(await fs.readFile(path.join(root, 'b.ts'), 'utf8')).toBe('close()'); + expect(await fs.readFile(path.join(root, 'c.ts'), 'utf8')).toBe('close();'); + expect((await fs.readdir(root)).sort()).toEqual(['a.ts', 'b.ts', 'c.ts']); + }, + ); + it('rejects lexical traversal and symlink escapes', async () => { const p = await plan({ 'a.ts': 'close()' }); p.edits[0].file_path = '../outside.ts'; @@ -171,22 +214,30 @@ describe('exact rename plan application', () => { expect((await fs.readdir(root)).sort()).toEqual(['a.ts', 'b.ts']); }); - it('reports a race after an earlier write as partial', async () => { - const p = await plan({ 'a.ts': 'close()', 'b.ts': 'close()' }); - const original = fs.rename.bind(fs); - vi.spyOn(fs, 'rename').mockImplementation(async (...args) => { - await original(...args); - if (String(args[1]).endsWith('a.ts')) - await fs.writeFile(path.join(root, 'b.ts'), 'user edit'); - }); - expect(await executeRenamePlan(root, p, false)).toMatchObject({ - status: 'partial', - applied: true, - total_edits: 1, - failed_files: ['b.ts'], - }); - expect(await fs.readFile(path.join(root, 'b.ts'), 'utf8')).toBe('user edit'); - }); + it.each(['a.ts', 'b.ts'])( + 'reports a race in %s after an earlier write as partial', + async (changedFile) => { + const p = await plan({ 'a.ts': 'close()', 'b.ts': 'close()' }); + const original = fs.rename.bind(fs); + vi.spyOn(fs, 'rename').mockImplementation(async (...args) => { + await original(...args); + if (String(args[1]).endsWith('a.ts')) + await fs.writeFile(path.join(root, changedFile), 'user edit'); + }); + expect(await executeRenamePlan(root, p, false)).toMatchObject({ + status: 'partial', + applied: true, + application_status: 'partial', + code: 'source_changed', + total_edits: 1, + failed_files: ['b.ts'], + }); + expect(await fs.readFile(path.join(root, changedFile), 'utf8')).toBe('user edit'); + if (changedFile === 'a.ts') + expect(await fs.readFile(path.join(root, 'b.ts'), 'utf8')).toBe('close()'); + expect((await fs.readdir(root)).sort()).toEqual(['a.ts', 'b.ts']); + }, + ); it('blocks a destination changed while staging its replacement', async () => { const p = await plan({ 'a.ts': 'close()' }); diff --git a/gitnexus/test/unit/rename-retention.test.ts b/gitnexus/test/unit/rename-retention.test.ts new file mode 100644 index 000000000..09f3ad3d3 --- /dev/null +++ b/gitnexus/test/unit/rename-retention.test.ts @@ -0,0 +1,170 @@ +/** Exercise rename dispatch and source policy with real metadata and semantic plans. */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +const { lbugMocks } = vi.hoisted(() => ({ + lbugMocks: { + initLbug: vi.fn().mockResolvedValue(undefined), + executeQuery: vi.fn().mockResolvedValue([]), + executeParameterized: vi.fn().mockResolvedValue([]), + ensureVectorExtension: vi.fn().mockResolvedValue(true), + closeLbug: vi.fn().mockResolvedValue(undefined), + isLbugReady: vi.fn().mockReturnValue(true), + }, +})); + +vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => ({ + ...(await importOriginal()), + ...lbugMocks, +})); +vi.mock('../../src/mcp/core/lbug-adapter.js', async (importOriginal) => ({ + ...(await importOriginal()), + ...lbugMocks, +})); +vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => ({ + ...(await importOriginal()), + listRegisteredRepos: vi.fn().mockResolvedValue([]), +})); +vi.mock('../../src/core/git-staleness.js', () => ({ + checkStaleness: vi.fn().mockReturnValue({ isStale: false, commitsBehind: 0 }), + checkStalenessAsync: vi.fn().mockResolvedValue({ isStale: false, commitsBehind: 0 }), + checkCwdMatch: vi.fn().mockResolvedValue({ match: 'none' }), +})); + +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; + +const NAME = 'rename-source-policy'; +const INDEXED_AT = '2024-06-01T12:00:00Z'; +const FILE = 'target.ts'; +const UID = `Function:${FILE}:target`; +const SOURCE = 'function target() { return "retention-private-body"; }\ntarget();\n'; +const symbol = { + id: UID, + name: 'target', + type: 'Function', + filePath: FILE, + startLine: 0, + endLine: 0, +}; +const routes = [ + { route: 'preview', tool: 'rename_preview', dry_run: false }, + { route: 'dry run', tool: 'rename', dry_run: true }, + { route: 'apply', tool: 'rename', dry_run: false }, +] as const; + +describe('rename checkout source policy', () => { + let root: string; + let checkout: string; + let storage: string; + let backend: LocalBackend; + + const writeMeta = async (contentRetention?: string) => { + await fs.writeFile( + path.join(storage, 'gitnexus.json'), + JSON.stringify({ contentRetention, indexedAt: INDEXED_AT }), + ); + }; + + beforeEach(async () => { + vi.clearAllMocks(); + root = await fs.mkdtemp(path.join(os.tmpdir(), 'gnx-rename-retention-')); + checkout = path.join(root, 'checkout'); + storage = path.join(root, 'index'); + await fs.mkdir(checkout); + await fs.mkdir(path.join(storage, 'lbug'), { recursive: true }); + await writeMeta('full'); + await fs.writeFile(path.join(checkout, FILE), SOURCE); + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: NAME, + path: checkout, + storagePath: storage, + indexedAt: INDEXED_AT, + lastCommit: 'abc123', + } as Awaited>[number], + ]); + lbugMocks.executeQuery.mockResolvedValue([ + { id: `File:${FILE}`, name: FILE, filePath: FILE }, + symbol, + ]); + // Context, policy, parser, plan and apply run normally against the graph rows. + lbugMocks.executeParameterized.mockImplementation(async (_db, query) => + query.includes('MATCH (n {id: $uid})') || query.includes('WHERE n.name = $symName') + ? [symbol] + : [], + ); + backend = new LocalBackend(); + }); + + afterEach(async () => { + await backend.disconnect(); + await fs.rm(root, { recursive: true, force: true }); + }); + + const rename = ({ tool, dry_run }: (typeof routes)[number]) => + backend.callTool(tool, { repo: NAME, symbol_uid: UID, new_name: 'renamed', dry_run }); + + const expectUnavailable = (result: any, reason: string) => { + expect(result).toMatchObject({ code: 'source-unavailable', reason }); + expect(Object.keys(result).sort()).toEqual(['code', 'error', 'reason']); + expect(result.changes).toBeUndefined(); + expect(JSON.stringify(result)).not.toContain('retention-private-body'); + expect(lbugMocks.executeQuery).not.toHaveBeenCalled(); + }; + + for (const retention of ['none', 'symbol', 'corrupt']) { + it.each(routes)( + `refuses $route source disclosure and writes with ${retention} retention`, + async (route) => { + await writeMeta(retention); + const result = await rename(route); + expectUnavailable(result, 'content-retention'); + expect(await fs.readFile(path.join(checkout, FILE), 'utf8')).toBe(SOURCE); + }, + ); + } + + it.each(routes)('permits $route under full retention with a live checkout', async (route) => { + const result = await rename(route); + expect(result.status).toBe('success'); + expect(result.total_edits).toBe(2); + expect(result.changes[0].edits[0].before).toContain('retention-private-body'); + const applied = route.tool === 'rename' && !route.dry_run; + expect(result.applied).toBe(applied); + expect(await fs.readFile(path.join(checkout, FILE), 'utf8')).toBe( + applied ? 'function renamed() { return "retention-private-body"; }\nrenamed();\n' : SOURCE, + ); + }); + + it.each(routes)('refuses $route when the full-retention checkout is missing', async (route) => { + await fs.rm(checkout, { recursive: true }); + expectUnavailable(await rename(route), 'checkout-missing'); + }); + + it('preserves missing-symbol argument validation under non-full retention', async () => { + await writeMeta('none'); + const result = await backend.callTool('rename_preview', { repo: NAME, new_name: 'renamed' }); + expect(result.error).toContain('Either symbol_name or symbol_uid'); + expect(result.code).toBeUndefined(); + }); + + it('preserves symbol ambiguity without exposing source under non-full retention', async () => { + await writeMeta('none'); + lbugMocks.executeParameterized.mockResolvedValue([ + symbol, + { ...symbol, id: 'Function:other.ts:target', filePath: 'other.ts' }, + ]); + const result = await backend.callTool('rename_preview', { + repo: NAME, + symbol_name: 'target', + new_name: 'renamed', + }); + expect(result.status).toBe('ambiguous'); + expect(result.candidates).toHaveLength(2); + expect(result.changes).toBeUndefined(); + expect(JSON.stringify(result)).not.toContain('retention-private-body'); + }); +});