scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-01 02:01:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

refactor(mcp): share one blank-uid check across impact, trace and symbol lookup (#3354)

Browse source 
The "trimmed uid, or omitted when blank/non-string" rule was inlined four
times, three of them trimming twice. nonBlankUid() owns it now; behaviour is
unchanged. The whitespace and empty target_uid tests collapse into one it.each.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergo Magyar 2026-09-25 18:38:36 +00:00
parent 04851737b4
commit 9aa2a93e00
2 changed files with 32 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -303,6 +303,15 @@ function resolveAliasString(canonical: unknown, legacy: unknown): string | undef
return undefined;
}
/**
* A `*_uid` param as a lookup key: trimmed, or `undefined` when it is blank or
* not a string (#3354). Strict adapters send `" "`/`""` for an omitted optional
* string, and the MCP envelope is not type-validated.
*/
function nonBlankUid(value: unknown): string | undefined {
return typeof value === 'string' ? value.trim() || undefined : undefined;
}
interface StringAliasDefinition {
canonical: string;
aliases: readonly string[];
@ -4451,11 +4460,10 @@ export class LocalBackend {
| { kind: 'not_found' }
> {
const { name, include_content } = query;
// A blank uid is an omitted optional field (strict adapters send " "/""),
// not a lookup key — fall through to the name instead of `not_found`.
// The MCP envelope is not type-validated: a non-string uid is also treated
// as omitted, matching normalizeToolParams' impact target_uid check.
const uid = typeof query.uid === 'string' ? query.uid.trim() : undefined;
// A blank or non-string uid is omitted, not a lookup key — fall through to
// the name instead of `not_found`, matching normalizeToolParams' impact
// target_uid check.
const uid = nonBlankUid(query.uid);
const selectClause = `n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine${include_content ? ', n.content AS content' : ''}`;
// Direct UID — zero-ambiguity path.
@ -6811,11 +6819,7 @@ export class LocalBackend {
if (fromOutcome.kind === 'not_found') {
return {
status: 'not_found',
error: `Source symbol '${
typeof params.from_uid === 'string' && params.from_uid.trim()
? params.from_uid.trim()
: params.from
}' not found.`,
error: `Source symbol '${nonBlankUid(params.from_uid) ?? params.from}' not found.`,
suggestion: 'Check the symbol name or use --from-uid for zero-ambiguity.',
};
}
@ -6842,11 +6846,7 @@ export class LocalBackend {
if (toOutcome.kind === 'not_found') {
return {
status: 'not_found',
error: `Target symbol '${
typeof params.to_uid === 'string' && params.to_uid.trim()
? params.to_uid.trim()
: params.to
}' not found.`,
error: `Target symbol '${nonBlankUid(params.to_uid) ?? params.to}' not found.`,
suggestion: 'Check the symbol name or use --to-uid for zero-ambiguity.',
};
}
@ -7226,10 +7226,7 @@ export class LocalBackend {
);
if (outcome.kind === 'not_found') {
const missing =
typeof params.target_uid === 'string' && params.target_uid.trim()
? params.target_uid.trim()
: target;
const missing = nonBlankUid(params.target_uid) ?? target;
// not_found = no resolved symbol, so the envelope keeps the partial-but-
// typed target (typed PdgImpactTarget — there is no id/type/filePath yet).
const notFoundTarget: PdgImpactTarget = { name: target };

44
gitnexus/test/unit/calltool-dispatch.test.ts
View file

@ -638,36 +638,24 @@ describe('LocalBackend.callTool', () => {
expect(result).not.toHaveProperty('byDepthCounts');
});
it('treats a whitespace target_uid as omitted and resolves the name (#3354)', async () => {
(executeParameterized as any).mockResolvedValue([]);
it.each([[' '], ['']])(
'treats a blank target_uid %j as omitted and resolves the name (#3354)',
async (targetUid) => {
(executeParameterized as any).mockResolvedValue([]);
const result = await backend.callTool('impact', {
target: 'validate',
target_uid: ' ',
direction: 'upstream',
});
const result = await backend.callTool('impact', {
target: 'validate',
target_uid: targetUid,
direction: 'upstream',
});
// Name resolution ran (no rows → not found by NAME), not a lookup of uid ' '.
expect(result.error).toBe("Target 'validate' not found");
const boundParams = (executeParameterized as any).mock.calls.map((c: unknown[]) => c[2]);
expect(boundParams).not.toContainEqual(expect.objectContaining({ uid: expect.anything() }));
expect(boundParams).toContainEqual(expect.objectContaining({ symName: 'validate' }));
});
it('treats an empty target_uid like a whitespace one and resolves the name (#3354)', async () => {
(executeParameterized as any).mockResolvedValue([]);
const result = await backend.callTool('impact', {
target: 'validate',
target_uid: '',
direction: 'upstream',
});
expect(result.error).toBe("Target 'validate' not found");
const boundParams = (executeParameterized as any).mock.calls.map((c: unknown[]) => c[2]);
expect(boundParams).not.toContainEqual(expect.objectContaining({ uid: expect.anything() }));
expect(boundParams).toContainEqual(expect.objectContaining({ symName: 'validate' }));
});
// Name resolution ran (no rows → not found by NAME), not a lookup of the blank uid.
expect(result.error).toBe("Target 'validate' not found");
const boundParams = (executeParameterized as any).mock.calls.map((c: unknown[]) => c[2]);
expect(boundParams).not.toContainEqual(expect.objectContaining({ uid: expect.anything() }));
expect(boundParams).toContainEqual(expect.objectContaining({ symName: 'validate' }));
},
);
it('treats a non-string impact target_uid as omitted instead of throwing (#3354)', async () => {
(executeParameterized as any).mockResolvedValue([]);