From f2379f73270aa1d6bcf2ddc55fdba03f2c086319 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Mon, 15 Jun 2026 19:19:09 +0200 Subject: [PATCH] fix(tools): register get_location in availableTools and refine location UX --- api/app/clients/tools/index.js | 4 +- api/app/clients/tools/manifest.json | 2 +- .../clients/tools/structured/GetLocation.js | 47 +++++++++++-------- .../tools/structured/GetLocation.spec.js | 18 ++++--- .../GetLocation.registration.spec.js | 19 ++++++++ api/app/clients/tools/util/handleTools.js | 7 ++- .../Nav/SettingsTabs/Personalization.tsx | 7 ++- .../__tests__/Personalization.spec.tsx | 31 ++++++++++++ client/src/utils/geocode.ts | 11 ++++- 9 files changed, 110 insertions(+), 36 deletions(-) create mode 100644 api/app/clients/tools/structured/__tests__/GetLocation.registration.spec.js diff --git a/api/app/clients/tools/index.js b/api/app/clients/tools/index.js index 6865b5ccdb..3563e12fc9 100644 --- a/api/app/clients/tools/index.js +++ b/api/app/clients/tools/index.js @@ -12,7 +12,7 @@ const TraversaalSearch = require('./structured/TraversaalSearch'); const createOpenAIImageTools = require('./structured/OpenAIImageTools'); const TavilySearchResults = require('./structured/TavilySearchResults'); const createGeminiImageTool = require('./structured/GeminiImageGen'); -const createLocationTool = require('./structured/GetLocation'); +const GetLocation = require('./structured/GetLocation'); module.exports = { ...manifest, @@ -28,5 +28,5 @@ module.exports = { TavilySearchResults, createOpenAIImageTools, createGeminiImageTool, - createLocationTool, + GetLocation, }; diff --git a/api/app/clients/tools/manifest.json b/api/app/clients/tools/manifest.json index b13932522c..d02a7345a9 100644 --- a/api/app/clients/tools/manifest.json +++ b/api/app/clients/tools/manifest.json @@ -171,7 +171,7 @@ "name": "Get Location", "pluginKey": "get_location", "description": "Returns the user's shared location (place, coordinates, timezone) so the assistant can tailor language, units, and regional context.", - "icon": "assets/google-search.svg", + "icon": "assets/logo.svg", "authConfig": [] } ] diff --git a/api/app/clients/tools/structured/GetLocation.js b/api/app/clients/tools/structured/GetLocation.js index 4f65332ee2..8f5771b5a6 100644 --- a/api/app/clients/tools/structured/GetLocation.js +++ b/api/app/clients/tools/structured/GetLocation.js @@ -1,4 +1,4 @@ -const { tool } = require('@librechat/agents/langchain/tools'); +const { Tool } = require('@librechat/agents/langchain/tools'); const { formatLocationToolResult } = require('@librechat/api'); const locationSchema = { @@ -8,22 +8,31 @@ const locationSchema = { }; /** - * Factory for the `get_location` tool, bound to the current request/user. - * @param {{ userId?: string, req?: import('express').Request }} params - * @returns {Promise} + * GetLocation - returns the user's shared location (place, coordinates, timezone). + * Reads the resolved app config (admin feature flag) and the user's stored + * `personalization.location` from the request, and delegates formatting to + * `formatLocationToolResult`. Gracefully reports when disabled or not shared. */ -module.exports = async function createLocationTool({ req } = {}) { - return tool( - async () => { - const featureEnabled = req?.config?.location?.enabled !== false; - const location = req?.user?.personalization?.location; - return formatLocationToolResult(location, { featureEnabled }); - }, - { - name: 'get_location', - description: - "Returns the user's current location (place, coordinates, timezone) when they have shared it. Use it to tailor language, regional context, units, or weather lookups.", - schema: locationSchema, - }, - ); -}; +class GetLocation extends Tool { + constructor(fields = {}) { + super(); + + /** @type {boolean} Used to initialize the Tool without request context. */ + this.override = fields.override ?? false; + this.req = fields.req; + this.userId = fields.userId; + + this.name = 'get_location'; + this.description = + "Returns the user's current location (place, coordinates, timezone) when they have shared it. Use it to tailor language, regional context, units, or weather lookups."; + this.schema = locationSchema; + } + + async _call() { + const featureEnabled = this.req?.config?.location?.enabled !== false; + const location = this.req?.user?.personalization?.location; + return formatLocationToolResult(location, { featureEnabled }); + } +} + +module.exports = GetLocation; diff --git a/api/app/clients/tools/structured/GetLocation.spec.js b/api/app/clients/tools/structured/GetLocation.spec.js index 8207557b66..0fc2be5f4e 100644 --- a/api/app/clients/tools/structured/GetLocation.spec.js +++ b/api/app/clients/tools/structured/GetLocation.spec.js @@ -1,14 +1,13 @@ -const createLocationTool = require('./GetLocation'); +const GetLocation = require('./GetLocation'); const makeReq = ({ location, featureEnabled = true } = {}) => ({ config: { location: { enabled: featureEnabled } }, user: { id: 'user-1', personalization: location ? { location } : {} }, }); -describe('createLocationTool', () => { +describe('GetLocation tool', () => { it('returns the user location when enabled', async () => { - const tool = await createLocationTool({ - userId: 'user-1', + const tool = new GetLocation({ req: makeReq({ location: { enabled: true, @@ -24,17 +23,22 @@ describe('createLocationTool', () => { }); it('returns a not-shared message when the user has not opted in', async () => { - const tool = await createLocationTool({ userId: 'user-1', req: makeReq({}) }); + const tool = new GetLocation({ req: makeReq({}) }); const result = await tool.invoke({}); expect(result).toMatch(/has not shared/i); }); it('returns a disabled message when the admin flag is off', async () => { - const tool = await createLocationTool({ - userId: 'user-1', + const tool = new GetLocation({ req: makeReq({ location: { enabled: true, manual: 'X' }, featureEnabled: false }), }); const result = await tool.invoke({}); expect(result).toMatch(/disabled/i); }); + + it('can be constructed without request context (override) for tool discovery', () => { + const tool = new GetLocation({ override: true }); + expect(tool.name).toBe('get_location'); + expect(tool.schema).toBeDefined(); + }); }); diff --git a/api/app/clients/tools/structured/__tests__/GetLocation.registration.spec.js b/api/app/clients/tools/structured/__tests__/GetLocation.registration.spec.js new file mode 100644 index 0000000000..bcd3a37918 --- /dev/null +++ b/api/app/clients/tools/structured/__tests__/GetLocation.registration.spec.js @@ -0,0 +1,19 @@ +const { Tool } = require('@librechat/agents/langchain/tools'); +const GetLocation = require('../GetLocation'); + +describe('get_location tool registration', () => { + it('GetLocation is a Tool subclass so loadAndFormatTools discovers it', () => { + expect(GetLocation.prototype instanceof Tool).toBe(true); + }); + + it('can be instantiated with override for discovery without request context', () => { + const tool = new GetLocation({ override: true }); + expect(tool.name).toBe('get_location'); + expect(tool.schema).toBeDefined(); + }); + + it('has a plain-object schema compatible with loadAndFormatTools (non-Zod)', () => { + const tool = new GetLocation({ override: true }); + expect(tool.schema).toEqual({ type: 'object', properties: {}, required: [] }); + }); +}); diff --git a/api/app/clients/tools/util/handleTools.js b/api/app/clients/tools/util/handleTools.js index 8cd9356085..7d2cd52cde 100644 --- a/api/app/clients/tools/util/handleTools.js +++ b/api/app/clients/tools/util/handleTools.js @@ -34,7 +34,7 @@ const { TavilySearchResults, createGeminiImageTool, createOpenAIImageTools, - createLocationTool, + GetLocation, } = require('../'); const { createMCPTool, @@ -182,6 +182,7 @@ const loadTools = async ({ google: GoogleSearchAPI, open_weather: OpenWeather, wolfram: StructuredWolfram, + get_location: GetLocation, 'stable-diffusion': StructuredSD, 'azure-ai-search': StructuredACS, traversaal_search: TraversaalSearch, @@ -231,9 +232,6 @@ const loadTools = async ({ fileStrategy, }); }, - get_location: async () => { - return createLocationTool({ userId: user, req: options.req }); - }, }; const requestedTools = {}; @@ -264,6 +262,7 @@ const loadTools = async ({ dalle: imageGenOptions, 'stable-diffusion': imageGenOptions, gemini_image_gen: imageGenOptions, + get_location: { req: options.req }, }; /** @type {Record} */ diff --git a/client/src/components/Nav/SettingsTabs/Personalization.tsx b/client/src/components/Nav/SettingsTabs/Personalization.tsx index 7d5211205f..824cde2c1d 100644 --- a/client/src/components/Nav/SettingsTabs/Personalization.tsx +++ b/client/src/components/Nav/SettingsTabs/Personalization.tsx @@ -73,7 +73,12 @@ export default function Personalization({ const handleLocationToggle = (checked: boolean) => { setLocationEnabled(checked); - persistLocation({ enabled: checked, source: 'manual', manual: manualLocation || undefined }); + const existing = user?.personalization?.location; + persistLocation({ + ...(existing ?? {}), + enabled: checked, + manual: manualLocation || existing?.manual, + }); }; const handleManualBlur = () => { diff --git a/client/src/components/Nav/SettingsTabs/__tests__/Personalization.spec.tsx b/client/src/components/Nav/SettingsTabs/__tests__/Personalization.spec.tsx index 0963f7c5ba..293093a641 100644 --- a/client/src/components/Nav/SettingsTabs/__tests__/Personalization.spec.tsx +++ b/client/src/components/Nav/SettingsTabs/__tests__/Personalization.spec.tsx @@ -6,9 +6,16 @@ import * as authQueries from '~/data-provider/Auth/queries'; import Personalization from '../Personalization'; const mockMutate = jest.fn(); +const mockShowToast = jest.fn(); + +jest.mock('@librechat/client', () => ({ + ...jest.requireActual('@librechat/client'), + useToastContext: () => ({ showToast: mockShowToast }), +})); beforeEach(() => { mockMutate.mockClear(); + mockShowToast.mockClear(); jest .spyOn(authQueries, 'useGetUserQuery') @@ -63,4 +70,28 @@ describe('Personalization location section', () => { expect.objectContaining({ manual: 'Tokyo, Japan', source: 'manual' }), ); }); + + it('shows a warning toast when geolocation is denied and does not call mutate with auto source', async () => { + const deniedError = Object.assign(new Error('denied'), { + code: 1, + PERMISSION_DENIED: 1, + }); + jest + .spyOn(global.navigator.geolocation, 'getCurrentPosition') + .mockImplementation((_success, error) => error?.(deniedError as GeolocationPositionError)); + + render( + , + ); + + const button = screen.getByText('Use my device location'); + fireEvent.click(button); + + await waitFor(() => expect(mockShowToast).toHaveBeenCalled()); + expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ status: 'warning' })); + + expect(mockMutate).not.toHaveBeenCalledWith(expect.objectContaining({ source: 'auto' })); + + expect(screen.getByLabelText('Set location manually')).toBeInTheDocument(); + }); }); diff --git a/client/src/utils/geocode.ts b/client/src/utils/geocode.ts index a8114ca94f..94d206e18f 100644 --- a/client/src/utils/geocode.ts +++ b/client/src/utils/geocode.ts @@ -2,6 +2,13 @@ const DEFAULT_ENDPOINT = 'https://api.bigdatacloud.net/data/reverse-geocode-clie const round = (n: number) => Math.round(n * 100) / 100; +interface BigDataCloudResponse { + city?: string; + locality?: string; + principalSubdivision?: string; + countryName?: string; +} + export interface ResolvedLocation { place?: string; coordinates: { latitude: number; longitude: number }; @@ -31,9 +38,9 @@ export async function reverseGeocode( if (!response.ok) { return { coordinates, timezone }; } - const data = await response.json(); + const data = (await response.json()) as BigDataCloudResponse; const place = [data.city || data.locality, data.principalSubdivision, data.countryName] - .filter((part: unknown): part is string => typeof part === 'string' && part.length > 0) + .filter((part): part is string => typeof part === 'string' && part.length > 0) .join(', '); return { place: place || undefined, coordinates, timezone }; } catch {