From 6f34c47b9ee03447a55a4f40a55a5b4da3766fe5 Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 29 Sep 2026 12:39:30 +0200 Subject: [PATCH] Quieter Redis, sharper marketplace search, clearer adapter install - Redis: without REDIS_URL no client is created. The localhost:6379 fallback made every default self-hosted install retry forever and log a warning every two seconds. Connection errors are now logged once per distinct error, with a line when the connection comes back; health tells 'not configured' apart from 'configured but unreachable'. - Marketplace search: every word must start a word (camelCase and letter/digit humps count). 'SAP' no longer returns WhatsApp, MessageBird and NewsAPI; 'api' still finds NewsAPI, 'hana' finds S/4HANA. - Install modal: adapters whose base URL is built from variables (SAP HANA) no longer offer 'Skip for now', which always failed with a 400 after the modal had closed. A failed import keeps the modal open and shows the error there. Code blocks in the instructions wrap instead of running past the edge. - Connector page: long endpoint paths (the SQL of database tools) collapse to one line with a 'Show full SQL' toggle. --- .../backend/src/common/redis.service.spec.ts | 74 +++++++++++++++++ packages/backend/src/common/redis.service.ts | 43 +++++++--- .../backend/src/health/health.controller.ts | 13 ++- .../frontend/src/app/connectors/[id]/page.tsx | 37 ++++++++- .../src/app/connectors/store/page.tsx | 81 +++++++++++++------ .../frontend/src/lib/marketplace-search.ts | 25 ++++++ 6 files changed, 235 insertions(+), 38 deletions(-) create mode 100644 packages/backend/src/common/redis.service.spec.ts create mode 100644 packages/frontend/src/lib/marketplace-search.ts diff --git a/packages/backend/src/common/redis.service.spec.ts b/packages/backend/src/common/redis.service.spec.ts new file mode 100644 index 00000000..8123ed90 --- /dev/null +++ b/packages/backend/src/common/redis.service.spec.ts @@ -0,0 +1,74 @@ +import { EventEmitter } from 'events'; +import { Logger } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; + +// A stand-in for ioredis: an event emitter with a connect() the test controls. +const instances: FakeRedis[] = []; +class FakeRedis extends EventEmitter { + status = 'wait'; + constructor( + public url: string, + public opts: unknown, + ) { + super(); + instances.push(this); + } + connect = jest.fn(async () => { + throw new Error('connect ECONNREFUSED 127.0.0.1:6379'); + }); +} +jest.mock('ioredis', () => ({ __esModule: true, default: FakeRedis })); + +import { RedisService } from './redis.service'; + +const config = (env: Record) => + ({ get: (key: string) => env[key] }) as unknown as ConfigService; + +describe('RedisService', () => { + let warn: jest.SpyInstance; + + beforeEach(() => { + instances.length = 0; + warn = jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); + jest.spyOn(Logger.prototype, 'log').mockImplementation(() => undefined); + }); + + afterEach(() => jest.restoreAllMocks()); + + it('creates no client and connects nowhere when REDIS_URL is not set', async () => { + const svc = new RedisService(config({})); + await svc.onModuleInit(); + + expect(instances).toHaveLength(0); + expect(svc.isConfigured).toBe(false); + expect(svc.isConnected).toBe(false); + expect(await svc.get('k')).toBeNull(); + expect(await svc.incr('k')).toBe(0); + await expect(svc.onModuleDestroy()).resolves.toBeUndefined(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('treats a blank REDIS_URL as not set', async () => { + const svc = new RedisService(config({ REDIS_URL: ' ' })); + await svc.onModuleInit(); + expect(instances).toHaveLength(0); + }); + + it('logs a repeated connection error once, and again after a recovery', async () => { + const svc = new RedisService(config({ REDIS_URL: 'redis://redis:6379' })); + await svc.onModuleInit(); + + expect(instances).toHaveLength(1); + expect(instances[0].url).toBe('redis://redis:6379'); + expect(svc.isConfigured).toBe(true); + warn.mockClear(); // the failed first connect() is reported on its own + + const refused = new Error('connect ECONNREFUSED 10.0.0.5:6379'); + for (let i = 0; i < 5; i++) instances[0].emit('error', refused); + expect(warn).toHaveBeenCalledTimes(1); + + instances[0].emit('ready'); + instances[0].emit('error', refused); + expect(warn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/backend/src/common/redis.service.ts b/packages/backend/src/common/redis.service.ts index fdc41f03..e6327dd1 100644 --- a/packages/backend/src/common/redis.service.ts +++ b/packages/backend/src/common/redis.service.ts @@ -10,26 +10,42 @@ import Redis from 'ioredis'; @Injectable() export class RedisService implements OnModuleInit, OnModuleDestroy { private readonly logger = new Logger(RedisService.name); - private client: Redis; + private client: Redis | null = null; + private lastError: string | null = null; constructor(private readonly configService: ConfigService) {} async onModuleInit() { - const url = this.configService.get('REDIS_URL', 'redis://localhost:6379'); + // Redis is optional. Without REDIS_URL there is nothing to connect to: + // falling back to localhost:6379 made ioredis retry forever on every + // default self-hosted install and log a warning every two seconds. + const url = this.configService.get('REDIS_URL')?.trim(); + if (!url) { + this.logger.log('REDIS_URL is not set: caching and rate-limit counters stay in memory.'); + return; + } + this.client = new Redis(url, { maxRetriesPerRequest: 3, lazyConnect: true, }); + // Log a connection error once, not on every retry; say so when it recovers. this.client.on('error', (err) => { + if (err.message === this.lastError) return; + this.lastError = err.message; this.logger.warn(`Redis connection error: ${err.message}`); }); + this.client.on('ready', () => { + if (this.lastError) this.logger.log('Redis connection restored'); + this.lastError = null; + }); try { await this.client.connect(); this.logger.log('Redis connected'); } catch (err: any) { - this.logger.warn(`Redis not available: ${err.message}. Caching disabled.`); + this.logger.warn(`Redis not available: ${err.message}. Caching disabled until it is.`); } } @@ -39,45 +55,50 @@ export class RedisService implements OnModuleInit, OnModuleDestroy { } } + /** REDIS_URL is set; the connection may still be down. */ + get isConfigured(): boolean { + return this.client !== null; + } + get isConnected(): boolean { return this.client?.status === 'ready'; } async get(key: string): Promise { if (!this.isConnected) return null; - return this.client.get(key); + return this.client!.get(key); } async set(key: string, value: string, ttlSeconds?: number): Promise { if (!this.isConnected) return; if (ttlSeconds) { - await this.client.set(key, value, 'EX', ttlSeconds); + await this.client!.set(key, value, 'EX', ttlSeconds); } else { - await this.client.set(key, value); + await this.client!.set(key, value); } } async del(key: string): Promise { if (!this.isConnected) return; - await this.client.del(key); + await this.client!.del(key); } async incr(key: string): Promise { if (!this.isConnected) return 0; - return this.client.incr(key); + return this.client!.incr(key); } async expire(key: string, ttlSeconds: number): Promise { if (!this.isConnected) return; - await this.client.expire(key, ttlSeconds); + await this.client!.expire(key, ttlSeconds); } async ttl(key: string): Promise { if (!this.isConnected) return -1; - return this.client.ttl(key); + return this.client!.ttl(key); } - getClient(): Redis { + getClient(): Redis | null { return this.client; } } diff --git a/packages/backend/src/health/health.controller.ts b/packages/backend/src/health/health.controller.ts index 22bcf680..56d57735 100644 --- a/packages/backend/src/health/health.controller.ts +++ b/packages/backend/src/health/health.controller.ts @@ -130,8 +130,15 @@ export class HealthController { if (this.redis.isConnected) { return { redis: { status: 'up' } }; } - // Redis is optional — report as up with a message so the health check - // does not fail when Redis is simply not configured. - return { redis: { status: 'up', message: 'Not configured (optional)' } }; + // Redis is optional: report up with a message so the health check does + // not fail, but tell "not configured" apart from "configured and down". + return { + redis: { + status: 'up', + message: this.redis.isConfigured + ? 'Configured but not reachable (caching disabled)' + : 'Not configured (optional)', + }, + }; } } diff --git a/packages/frontend/src/app/connectors/[id]/page.tsx b/packages/frontend/src/app/connectors/[id]/page.tsx index 4b37c300..44ea26a0 100644 --- a/packages/frontend/src/app/connectors/[id]/page.tsx +++ b/packages/frontend/src/app/connectors/[id]/page.tsx @@ -1663,7 +1663,7 @@ export default function ConnectorDetailPage() { {/* Show mapping summary */}
{tool.endpointMapping?.path && ( - {tool.endpointMapping.path} + )} {tool.parameters?.properties && (() => { const allParams = Object.keys(tool.parameters.properties); @@ -1944,3 +1944,38 @@ function normalizeTokenAuthMethod(method: string | undefined): string { if (!method || method === 'post') return 'client_secret_post'; return method; } + +/** + * The endpoint line on a tool card. For REST it is a short path; for database + * tools it is the whole SQL statement, which printed in full, wrapped mid-word + * at `break-all`, made one card as tall as the screen (the SAP HANA tools run + * to 40 lines). Long ones show their first line and open on request. + */ +function ToolPathSummary({ path }: { path: string }) { + const [open, setOpen] = useState(false); + const oneLine = path.replace(/\s+/g, ' ').trim(); + if (oneLine.length <= 120 && !path.includes('\n')) { + return {path}; + } + return ( + + {open ? ( + 
+          {path.trim()}
+
+ ) : ( + + {oneLine} + + )} + + + ); +} diff --git a/packages/frontend/src/app/connectors/store/page.tsx b/packages/frontend/src/app/connectors/store/page.tsx index b285830b..9af97e60 100644 --- a/packages/frontend/src/app/connectors/store/page.tsx +++ b/packages/frontend/src/app/connectors/store/page.tsx @@ -13,6 +13,7 @@ import { Button, buttonVariants } from '@/components/ui/button'; import { Badge } from '@/components/ui/badge'; import { authTypeLabel, cn } from '@/lib/utils'; import { McpAssignModal } from '@/components/mcp-assign-modal'; +import { matchesSearch } from '@/lib/marketplace-search'; const REGION_LABELS: Record = { de: 'Germany', @@ -243,6 +244,16 @@ function seedOptionalCredentials(adapter: { ); } +/** + * Env vars the connector's base URL is built from (e.g. SAP_HANA_HOST in + * hana://{{SAP_HANA_HOST}}:{{SAP_HANA_PORT}}/). The connector cannot be + * created without them, so they cannot be skipped like an API key. + */ +function addressVars(adapter: { connector?: { baseUrl?: string } }): string[] { + const url = adapter.connector?.baseUrl ?? ''; + return [...new Set([...url.matchAll(/\{\{\s*([A-Za-z0-9_]+)\s*\}\}/g)].map((m) => m[1]))]; +} + interface AdapterDetail extends AdapterItem { // Long-form, Markdown-formatted help authored on the adapter JSON. // Rendered inside the install modal so users see "where to find your @@ -285,6 +296,9 @@ function AdapterStoreContent() { // toggles only its own field. Reset together with credentialValues. const [revealedCredentials, setRevealedCredentials] = useState>({}); const [configLoading, setConfigLoading] = useState(false); + // An import that fails from the modal keeps the modal open and shows why + // there, instead of closing it and leaving a banner at the top of the page. + const [configError, setConfigError] = useState(''); // MCP assignment modal state const [importedConnector, setImportedConnector] = useState<{ id: string; name: string } | null>(null); @@ -305,17 +319,19 @@ function AdapterStoreContent() { if (!token) return; setImporting(slug); setMsg(''); - setConfigAdapter(null); + setConfigError(''); try { const adapter = list.find((a) => a.slug === slug); const result = await adapters.import(slug, token, credentials); + setConfigAdapter(null); setMsg(describeImport(result.message, result.probe)); setImporting(null); // Show MCP assignment modal setImportedConnector({ id: result.connectorId, name: adapter?.name || slug }); } catch (err: any) { - setMsg(`Import failed: ${err.message}`); setImporting(null); + if (configAdapter) setConfigError(err.message); + else setMsg(`Import failed: ${err.message}`); } }; @@ -335,6 +351,7 @@ function AdapterStoreContent() { } // Fetch full adapter detail to show in modal + setConfigError(''); setConfigLoading(true); try { const detail = await adapters.get(adapter.slug, token); @@ -407,18 +424,20 @@ function AdapterStoreContent() { const filtered = list.filter((a) => { if (activeCategory && a.category !== activeCategory) return false; - if (!search) return true; - const q = search.toLowerCase(); + if (!search.trim()) return true; // Match what the card actually says, not just the stored slug: the card // reads "GERMANY" and "E-commerce", so those are the words people type. - return ( - a.name.toLowerCase().includes(q) || - a.description.toLowerCase().includes(q) || - a.slug.toLowerCase().includes(q) || - a.category?.toLowerCase().includes(q) || - categoryLabel(a.category ?? '').toLowerCase().includes(q) || - a.region?.toLowerCase().includes(q) || - (REGION_LABELS[a.region] ?? '').toLowerCase().includes(q) + return matchesSearch( + [ + a.name, + a.description, + a.slug, + a.category ?? '', + categoryLabel(a.category ?? ''), + a.region ?? '', + REGION_LABELS[a.region] ?? '', + ], + search, ); }); @@ -705,7 +724,9 @@ function AdapterStoreContent() { Configure {configAdapter.name}

- This adapter requires credentials to work. Enter them now or skip and configure later. + {addressVars(configAdapter).length > 0 + ? `${addressVars(configAdapter).map(formatEnvVarLabel).join(', ')} ${addressVars(configAdapter).length === 1 ? 'is' : 'are'} part of this connector's address, so enter ${addressVars(configAdapter).length === 1 ? 'it' : 'them'} now. The rest can wait.` + : 'This adapter requires credentials to work. Enter them now or skip and configure later.'}

@@ -722,7 +743,7 @@ function AdapterStoreContent() { 📖 How to get these credentials -
+
{configAdapter.instructions} @@ -796,19 +817,33 @@ function AdapterStoreContent() {
+ {configError && ( +

+ {configError} +

+ )} + {/* Pinned footer — always visible, never scrolls out of reach. */}
- + {/* Skipping only works when the address has no variables: the + backend cannot create a connector whose URL is still + {{SAP_HANA_HOST}}, and used to answer 400 after the modal + had already closed. */} + {addressVars(configAdapter).length === 0 && ( + + )} diff --git a/packages/frontend/src/lib/marketplace-search.ts b/packages/frontend/src/lib/marketplace-search.ts new file mode 100644 index 00000000..9bd238a8 --- /dev/null +++ b/packages/frontend/src/lib/marketplace-search.ts @@ -0,0 +1,25 @@ +/** + * Every word of the query must start a word somewhere in the fields. + * + * A plain substring match turned "SAP" into WhatsApp, MessageBird ("WhatsApp + * via Bird") and NewsAPI. Word starts include camelCase and letter/digit + * humps, so "api" still finds NewsAPI and "hana" finds S/4HANA, while the + * text as written still matches too ("4hana", "s/4hana"). + */ +export function matchesSearch(fields: string[], query: string): boolean { + const words = query.toLowerCase().split(/\s+/).filter(Boolean); + if (words.length === 0) return true; + const haystack = fields + .flatMap((f) => [ + f.toLowerCase(), + f + .replace(/([a-z])([A-Z])/g, '$1 $2') + .replace(/([A-Za-z])([0-9])|([0-9])([A-Za-z])/g, '$1$3 $2$4') + .toLowerCase(), + ]) + .join(' \n '); + return words.every((w) => { + const escaped = w.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return new RegExp(`(^|[^a-z0-9])${escaped}`).test(haystack); + }); +}