mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-03 02:21:44 +00:00
fix(mcp): bind impact BFS query filters as parameters (U3, #1907)
The impact blast-radius BFS built its n.id/r.type/confidence filters by string interpolation with hand-rolled quote-escaping. Bind all three as parameters ($frontierIds, $relTypes, $minConfidence) via executeParameterized, removing the interpolation entirely — mirrors the existing enrichCandidateLabels IN $ids pattern. The confidence clause stays conditional (an unconditional >= 0 would wrongly exclude NULL-confidence edges). Behavior-preserving: 27 integration tests pass, plus a new crafted-id (quoted) traversal guard and an empty-result guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
4f14a58f99
commit
bb110aca91
2 changed files with 82 additions and 7 deletions
|
|
@ -2920,8 +2920,14 @@ export class LocalBackend {
|
|||
typeof opts.offset === 'number' && Number.isFinite(opts.offset) ? opts.offset : 0;
|
||||
const paginationOffset = Math.max(0, Math.trunc(rawOffset));
|
||||
const summaryOnly = opts.summaryOnly ?? false;
|
||||
const relTypeFilter = relationTypes.map((t) => `'${t}'`).join(', ');
|
||||
const confidenceFilter = minConfidence > 0 ? ` AND r.confidence >= ${minConfidence}` : '';
|
||||
// Bind the BFS frontier query's filters as parameters (#1907 review F5):
|
||||
// node ids and relation types as bound lists, the confidence floor as a
|
||||
// bound number — no string interpolation reaches the query text. Preserve
|
||||
// the original "no confidence clause when minConfidence <= 0" behavior: an
|
||||
// unconditional `>= 0` would wrongly exclude NULL-confidence edges that the
|
||||
// unfiltered query includes.
|
||||
const safeMinConfidence = Number.isFinite(minConfidence) ? minConfidence : 0;
|
||||
const confidenceFilter = safeMinConfidence > 0 ? ' AND r.confidence >= $minConfidence' : '';
|
||||
|
||||
const symId = sym.id || sym[0];
|
||||
|
||||
|
|
@ -3009,15 +3015,19 @@ export class LocalBackend {
|
|||
for (let depth = 1; depth <= maxDepth && frontier.length > 0; depth++) {
|
||||
const nextFrontier: string[] = [];
|
||||
|
||||
// Batch frontier nodes into a single Cypher query per depth level
|
||||
const idList = frontier.map((id) => `'${id.replace(/'/g, "''")}'`).join(', ');
|
||||
// Batch frontier nodes into a single Cypher query per depth level.
|
||||
// ids/types/confidence are bound parameters (see above) — no interpolation.
|
||||
const query =
|
||||
direction === 'upstream'
|
||||
? `MATCH (caller)-[r:CodeRelation]->(n) WHERE n.id IN [${idList}] AND r.type IN [${relTypeFilter}]${confidenceFilter} RETURN n.id AS sourceId, caller.id AS id, caller.name AS name, labels(caller)[0] AS type, caller.filePath AS filePath, r.type AS relType, r.confidence AS confidence`
|
||||
: `MATCH (n)-[r:CodeRelation]->(callee) WHERE n.id IN [${idList}] AND r.type IN [${relTypeFilter}]${confidenceFilter} RETURN n.id AS sourceId, callee.id AS id, callee.name AS name, labels(callee)[0] AS type, callee.filePath AS filePath, r.type AS relType, r.confidence AS confidence`;
|
||||
? `MATCH (caller)-[r:CodeRelation]->(n) WHERE n.id IN $frontierIds AND r.type IN $relTypes${confidenceFilter} RETURN n.id AS sourceId, caller.id AS id, caller.name AS name, labels(caller)[0] AS type, caller.filePath AS filePath, r.type AS relType, r.confidence AS confidence`
|
||||
: `MATCH (n)-[r:CodeRelation]->(callee) WHERE n.id IN $frontierIds AND r.type IN $relTypes${confidenceFilter} RETURN n.id AS sourceId, callee.id AS id, callee.name AS name, labels(callee)[0] AS type, callee.filePath AS filePath, r.type AS relType, r.confidence AS confidence`;
|
||||
|
||||
try {
|
||||
const related = await executeQuery(repo.id, query);
|
||||
const related = await executeParameterized(repo.id, query, {
|
||||
frontierIds: frontier,
|
||||
relTypes: relationTypes,
|
||||
...(safeMinConfidence > 0 ? { minConfidence: safeMinConfidence } : {}),
|
||||
});
|
||||
|
||||
for (const rel of related) {
|
||||
const relId = rel.id || rel[1];
|
||||
|
|
|
|||
|
|
@ -437,3 +437,68 @@ withTestLbugDB(
|
|||
},
|
||||
},
|
||||
);
|
||||
|
||||
// ─── impact BFS bound parameters (#1907 review F5) ───────────────────────
|
||||
// Isolated DB (not the shared seed) with a frontier node whose id contains a
|
||||
// single quote. Under the old string-interpolated query this id had to be
|
||||
// hand-escaped; the parameterized query (executeParameterized with bound
|
||||
// $frontierIds/$relTypes) carries it as data. Guards that a quote-bearing id
|
||||
// traverses without a Prepare/parser error, and that a no-caller symbol
|
||||
// returns an empty result rather than erroring.
|
||||
withTestLbugDB(
|
||||
'local-backend-impact-param',
|
||||
(handle) => {
|
||||
describe('impact BFS bound parameters (#1907 F5)', () => {
|
||||
let backend: LocalBackend;
|
||||
|
||||
beforeAll(() => {
|
||||
const ext = handle as typeof handle & { _backend?: LocalBackend };
|
||||
if (!ext._backend) {
|
||||
throw new Error('LocalBackend not initialized — afterSetup did not attach _backend');
|
||||
}
|
||||
backend = ext._backend;
|
||||
});
|
||||
|
||||
it('traverses a caller whose id contains a single quote without a query error', async () => {
|
||||
const result = await backend.callTool('impact', { target: 'sink', direction: 'upstream' });
|
||||
expect(result).not.toHaveProperty('error');
|
||||
const d1 = result.byDepth?.[1] || result.byDepth?.['1'] || [];
|
||||
const callerIds = d1.map((d: any) => d.uid ?? d.id);
|
||||
expect(callerIds).toContain("func:o'd");
|
||||
});
|
||||
|
||||
it('returns an empty result (not an error) for a symbol with no callers', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'sink',
|
||||
direction: 'downstream',
|
||||
});
|
||||
expect(result).not.toHaveProperty('error');
|
||||
expect(result.impactedCount).toBe(0);
|
||||
});
|
||||
});
|
||||
},
|
||||
{
|
||||
seed: [
|
||||
`CREATE (a:Function {id: "func:o'd", name: 'odd', filePath: 'src/q.ts', startLine: 1, endLine: 3, isExported: true, content: 'function odd() {}', description: 'caller with a quote in its id'})`,
|
||||
`CREATE (b:Function {id: 'func:sink', name: 'sink', filePath: 'src/q.ts', startLine: 5, endLine: 8, isExported: true, content: 'function sink() {}', description: 'callee'})`,
|
||||
`MATCH (a:Function), (b:Function) WHERE a.id = "func:o'd" AND b.id = 'func:sink'
|
||||
CREATE (a)-[:CodeRelation {type: 'CALLS', confidence: 1.0, reason: 'direct', step: 0}]->(b)`,
|
||||
],
|
||||
poolAdapter: true,
|
||||
afterSetup: async (handle) => {
|
||||
vi.mocked(listRegisteredRepos).mockResolvedValue([
|
||||
{
|
||||
name: 'param-repo',
|
||||
path: '/param/repo',
|
||||
storagePath: handle.tmpHandle.dbPath,
|
||||
indexedAt: new Date().toISOString(),
|
||||
lastCommit: 'abc123',
|
||||
stats: { files: 1, nodes: 2, communities: 0, processes: 0 },
|
||||
},
|
||||
]);
|
||||
const backend = new LocalBackend();
|
||||
await backend.init();
|
||||
(handle as any)._backend = backend;
|
||||
},
|
||||
},
|
||||
);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue