diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 10e7b9996..85f13130d 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -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 }; diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 719f8b855..85fbbccea 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -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([]);