🖼️ refactor: Enhance Env Extraction & Agent Image Handling (#6131)

* refactor: use new image output format for agents using DALL-E tools

* refactor: Enhance image fetching with proxy support and adjust logging placement in DALL-E 3 integration

* refactor: Enhance StableDiffusionAPI to support agent-specific return values and display message for generated images

* refactor: Add unit test execution for librechat-mcp in backend review workflow

* refactor: Update environment variable extraction logic, export from serpate module to avoid circular refs, and remove deprecated tests

* refactor: Add unit tests for environment variable extraction and enhance StdioOptionsSchema to process env variables

.github/workflows/backend-review.yml

@@ -62,3 +62,6 @@ jobs:
 
       - name: Run librechat-data-provider unit tests
         run: cd packages/data-provider && npm run test:ci
+
+      - name: Run librechat-mcp unit tests
+        run: cd packages/mcp && npm run test:ci

api/app/clients/tools/structured/DALLE3.js

@@ -1,14 +1,17 @@
 const { z } = require('zod');
 const path = require('path');
 const OpenAI = require('openai');
+const fetch = require('node-fetch');
 const { v4: uuidv4 } = require('uuid');
 const { Tool } = require('@langchain/core/tools');
 const { HttpsProxyAgent } = require('https-proxy-agent');
-const { FileContext } = require('librechat-data-provider');
+const { FileContext, ContentTypes } = require('librechat-data-provider');
 const { getImageBasename } = require('~/server/services/Files/images');
 const extractBaseURL = require('~/utils/extractBaseURL');
 const { logger } = require('~/config');
 
+const displayMessage =
+  'DALL-E displayed an image. All generated images are already plainly visible, so don\'t repeat the descriptions in detail. Do not list download links as they are available in the UI already. The user may download the images by clicking on them, but do not mention anything about downloading to the user.';
 class DALLE3 extends Tool {
   constructor(fields = {}) {
     super();
@@ -114,10 +117,7 @@ class DALLE3 extends Tool {
     if (this.isAgent === true && typeof value === 'string') {
       return [value, {}];
     } else if (this.isAgent === true && typeof value === 'object') {
-      return [
-        'DALL-E displayed an image. All generated images are already plainly visible, so don\'t repeat the descriptions in detail. Do not list download links as they are available in the UI already. The user may download the images by clicking on them, but do not mention anything about downloading to the user.',
-        value,
-      ];
+      return [displayMessage, value];
     }
 
     return value;
@@ -160,6 +160,32 @@ Error Message: ${error.message}`);
       );
     }
 
+    if (this.isAgent) {
+      let fetchOptions = {};
+      if (process.env.PROXY) {
+        fetchOptions.agent = new HttpsProxyAgent(process.env.PROXY);
+      }
+      const imageResponse = await fetch(theImageUrl, fetchOptions);
+      const arrayBuffer = await imageResponse.arrayBuffer();
+      const base64 = Buffer.from(arrayBuffer).toString('base64');
+      const content = [
+        {
+          type: ContentTypes.IMAGE_URL,
+          image_url: {
+            url: `data:image/jpeg;base64,${base64}`,
+          },
+        },
+      ];
+
+      const response = [
+        {
+          type: ContentTypes.TEXT,
+          text: displayMessage,
+        },
+      ];
+      return [response, { content }];
+    }
+
     const imageBasename = getImageBasename(theImageUrl);
     const imageExt = path.extname(imageBasename);
 

api/app/clients/tools/structured/StableDiffusion.js

@@ -6,10 +6,13 @@ const axios = require('axios');
 const sharp = require('sharp');
 const { v4: uuidv4 } = require('uuid');
 const { Tool } = require('@langchain/core/tools');
-const { FileContext } = require('librechat-data-provider');
+const { FileContext, ContentTypes } = require('librechat-data-provider');
 const paths = require('~/config/paths');
 const { logger } = require('~/config');
 
+const displayMessage =
+  'Stable Diffusion displayed an image. All generated images are already plainly visible, so don\'t repeat the descriptions in detail. Do not list download links as they are available in the UI already. The user may download the images by clicking on them, but do not mention anything about downloading to the user.';
+
 class StableDiffusionAPI extends Tool {
   constructor(fields) {
     super();
@@ -21,6 +24,8 @@ class StableDiffusionAPI extends Tool {
     this.override = fields.override ?? false;
     /** @type {boolean} Necessary for output to contain all image metadata. */
     this.returnMetadata = fields.returnMetadata ?? false;
+    /** @type {boolean} */
+    this.isAgent = fields.isAgent;
     if (fields.uploadImageBuffer) {
       /** @type {uploadImageBuffer} Necessary for output to contain all image metadata. */
       this.uploadImageBuffer = fields.uploadImageBuffer.bind(this);
@@ -66,6 +71,16 @@ class StableDiffusionAPI extends Tool {
     return `![generated image](/${imageUrl})`;
   }
 
+  returnValue(value) {
+    if (this.isAgent === true && typeof value === 'string') {
+      return [value, {}];
+    } else if (this.isAgent === true && typeof value === 'object') {
+      return [displayMessage, value];
+    }
+
+    return value;
+  }
+
   getServerURL() {
     const url = process.env.SD_WEBUI_URL || '';
     if (!url && !this.override) {
@@ -113,6 +128,25 @@ class StableDiffusionAPI extends Tool {
     }
 
     try {
+      if (this.isAgent) {
+        const content = [
+          {
+            type: ContentTypes.IMAGE_URL,
+            image_url: {
+              url: `data:image/png;base64,${image}`,
+            },
+          },
+        ];
+
+        const response = [
+          {
+            type: ContentTypes.TEXT,
+            text: displayMessage,
+          },
+        ];
+        return [response, { content }];
+      }
+
       const buffer = Buffer.from(image.split(',', 1)[0], 'base64');
       if (this.returnMetadata && this.uploadImageBuffer && this.req) {
         const file = await this.uploadImageBuffer({
@@ -154,7 +188,7 @@ class StableDiffusionAPI extends Tool {
       logger.error('[StableDiffusion] Error while saving the image:', error);
     }
 
-    return this.result;
+    return this.returnValue(this.result);
   }
 }
 

api/server/controllers/agents/callbacks.js

@@ -1,4 +1,5 @@
-const { Tools, StepTypes, imageGenTools, FileContext } = require('librechat-data-provider');
+const { nanoid } = require('nanoid');
+const { Tools, StepTypes, FileContext } = require('librechat-data-provider');
 const {
   EnvVar,
   Providers,
@@ -242,32 +243,6 @@ function createToolEndCallback({ req, res, artifactPromises }) {
       return;
     }
 
-    if (imageGenTools.has(output.name)) {
-      artifactPromises.push(
-        (async () => {
-          const fileMetadata = Object.assign(output.artifact, {
-            messageId: metadata.run_id,
-            toolCallId: output.tool_call_id,
-            conversationId: metadata.thread_id,
-          });
-          if (!res.headersSent) {
-            return fileMetadata;
-          }
-
-          if (!fileMetadata) {
-            return null;
-          }
-
-          res.write(`event: attachment\ndata: ${JSON.stringify(fileMetadata)}\n\n`);
-          return fileMetadata;
-        })().catch((error) => {
-          logger.error('Error processing code output:', error);
-          return null;
-        }),
-      );
-      return;
-    }
-
     if (output.artifact.content) {
       /** @type {FormattedContent[]} */
       const content = output.artifact.content;
@@ -278,7 +253,7 @@ function createToolEndCallback({ req, res, artifactPromises }) {
         const { url } = part.image_url;
         artifactPromises.push(
           (async () => {
-            const filename = `${output.tool_call_id}-image-${new Date().getTime()}`;
+            const filename = `${output.name}_${output.tool_call_id}_img_${nanoid()}`;
             const file = await saveBase64Image(url, {
               req,
               filename,

package-lock.json

@@ -41798,7 +41798,7 @@
     },
     "packages/data-provider": {
       "name": "librechat-data-provider",
-      "version": "0.7.6994",
+      "version": "0.7.6995",
       "license": "ISC",
       "dependencies": {
         "axios": "^1.7.7",

packages/data-provider/package.json

@@ -1,6 +1,6 @@
 {
   "name": "librechat-data-provider",
-  "version": "0.7.6994",
+  "version": "0.7.6995",
   "description": "data services for librechat apps",
   "main": "dist/index.js",
   "module": "dist/index.es.js",

packages/data-provider/specs/mcp.spec.ts

@@ -0,0 +1,52 @@
+import { StdioOptionsSchema } from '../src/mcp';
+
+describe('Environment Variable Extraction (MCP)', () => {
+  const originalEnv = process.env;
+
+  beforeEach(() => {
+    process.env = {
+      ...originalEnv,
+      TEST_API_KEY: 'test-api-key-value',
+      ANOTHER_SECRET: 'another-secret-value',
+    };
+  });
+
+  afterEach(() => {
+    process.env = originalEnv;
+  });
+
+  describe('StdioOptionsSchema', () => {
+    it('should transform environment variables in the env field', () => {
+      const options = {
+        command: 'node',
+        args: ['server.js'],
+        env: {
+          API_KEY: '${TEST_API_KEY}',
+          ANOTHER_KEY: '${ANOTHER_SECRET}',
+          PLAIN_VALUE: 'plain-value',
+          NON_EXISTENT: '${NON_EXISTENT_VAR}',
+        },
+      };
+
+      const result = StdioOptionsSchema.parse(options);
+
+      expect(result.env).toEqual({
+        API_KEY: 'test-api-key-value',
+        ANOTHER_KEY: 'another-secret-value',
+        PLAIN_VALUE: 'plain-value',
+        NON_EXISTENT: '${NON_EXISTENT_VAR}',
+      });
+    });
+
+    it('should handle undefined env field', () => {
+      const options = {
+        command: 'node',
+        args: ['server.js'],
+      };
+
+      const result = StdioOptionsSchema.parse(options);
+
+      expect(result.env).toBeUndefined();
+    });
+  });
+});

packages/data-provider/specs/parsers.spec.ts

@@ -1,48 +0,0 @@
-import { extractEnvVariable } from '../src/parsers';
-
-describe('extractEnvVariable', () => {
-  const originalEnv = process.env;
-
-  beforeEach(() => {
-    jest.resetModules();
-    process.env = { ...originalEnv };
-  });
-
-  afterAll(() => {
-    process.env = originalEnv;
-  });
-
-  test('should return the value of the environment variable', () => {
-    process.env.TEST_VAR = 'test_value';
-    expect(extractEnvVariable('${TEST_VAR}')).toBe('test_value');
-  });
-
-  test('should return the original string if the envrionment variable is not defined correctly', () => {
-    process.env.TEST_VAR = 'test_value';
-    expect(extractEnvVariable('${ TEST_VAR }')).toBe('${ TEST_VAR }');
-  });
-
-  test('should return the original string if environment variable is not set', () => {
-    expect(extractEnvVariable('${NON_EXISTENT_VAR}')).toBe('${NON_EXISTENT_VAR}');
-  });
-
-  test('should return the original string if it does not contain an environment variable', () => {
-    expect(extractEnvVariable('some_string')).toBe('some_string');
-  });
-
-  test('should handle empty strings', () => {
-    expect(extractEnvVariable('')).toBe('');
-  });
-
-  test('should handle strings without variable format', () => {
-    expect(extractEnvVariable('no_var_here')).toBe('no_var_here');
-  });
-
-  test('should not process multiple variable formats', () => {
-    process.env.FIRST_VAR = 'first';
-    process.env.SECOND_VAR = 'second';
-    expect(extractEnvVariable('${FIRST_VAR} and ${SECOND_VAR}')).toBe(
-      '${FIRST_VAR} and ${SECOND_VAR}',
-    );
-  });
-});

packages/data-provider/specs/utils.spec.ts

@@ -0,0 +1,129 @@
+import { extractEnvVariable } from '../src/utils';
+
+describe('Environment Variable Extraction', () => {
+  const originalEnv = process.env;
+
+  beforeEach(() => {
+    process.env = {
+      ...originalEnv,
+      TEST_API_KEY: 'test-api-key-value',
+      ANOTHER_SECRET: 'another-secret-value',
+    };
+  });
+
+  afterEach(() => {
+    process.env = originalEnv;
+  });
+
+  describe('extractEnvVariable (original tests)', () => {
+    test('should return the value of the environment variable', () => {
+      process.env.TEST_VAR = 'test_value';
+      expect(extractEnvVariable('${TEST_VAR}')).toBe('test_value');
+    });
+
+    test('should return the original string if the envrionment variable is not defined correctly', () => {
+      process.env.TEST_VAR = 'test_value';
+      expect(extractEnvVariable('${ TEST_VAR }')).toBe('${ TEST_VAR }');
+    });
+
+    test('should return the original string if environment variable is not set', () => {
+      expect(extractEnvVariable('${NON_EXISTENT_VAR}')).toBe('${NON_EXISTENT_VAR}');
+    });
+
+    test('should return the original string if it does not contain an environment variable', () => {
+      expect(extractEnvVariable('some_string')).toBe('some_string');
+    });
+
+    test('should handle empty strings', () => {
+      expect(extractEnvVariable('')).toBe('');
+    });
+
+    test('should handle strings without variable format', () => {
+      expect(extractEnvVariable('no_var_here')).toBe('no_var_here');
+    });
+
+    /** No longer the expected behavior; keeping for reference */
+    test.skip('should not process multiple variable formats', () => {
+      process.env.FIRST_VAR = 'first';
+      process.env.SECOND_VAR = 'second';
+      expect(extractEnvVariable('${FIRST_VAR} and ${SECOND_VAR}')).toBe(
+        '${FIRST_VAR} and ${SECOND_VAR}',
+      );
+    });
+  });
+
+  describe('extractEnvVariable function', () => {
+    it('should extract environment variables from exact matches', () => {
+      expect(extractEnvVariable('${TEST_API_KEY}')).toBe('test-api-key-value');
+      expect(extractEnvVariable('${ANOTHER_SECRET}')).toBe('another-secret-value');
+    });
+
+    it('should extract environment variables from strings with prefixes', () => {
+      expect(extractEnvVariable('prefix-${TEST_API_KEY}')).toBe('prefix-test-api-key-value');
+    });
+
+    it('should extract environment variables from strings with suffixes', () => {
+      expect(extractEnvVariable('${TEST_API_KEY}-suffix')).toBe('test-api-key-value-suffix');
+    });
+
+    it('should extract environment variables from strings with both prefixes and suffixes', () => {
+      expect(extractEnvVariable('prefix-${TEST_API_KEY}-suffix')).toBe(
+        'prefix-test-api-key-value-suffix',
+      );
+    });
+
+    it('should not match invalid patterns', () => {
+      expect(extractEnvVariable('$TEST_API_KEY')).toBe('$TEST_API_KEY');
+      expect(extractEnvVariable('{TEST_API_KEY}')).toBe('{TEST_API_KEY}');
+      expect(extractEnvVariable('TEST_API_KEY')).toBe('TEST_API_KEY');
+    });
+  });
+
+  describe('extractEnvVariable', () => {
+    it('should extract environment variable values', () => {
+      expect(extractEnvVariable('${TEST_API_KEY}')).toBe('test-api-key-value');
+      expect(extractEnvVariable('${ANOTHER_SECRET}')).toBe('another-secret-value');
+    });
+
+    it('should return the original string if environment variable is not found', () => {
+      expect(extractEnvVariable('${NON_EXISTENT_VAR}')).toBe('${NON_EXISTENT_VAR}');
+    });
+
+    it('should return the original string if no environment variable pattern is found', () => {
+      expect(extractEnvVariable('plain-string')).toBe('plain-string');
+    });
+  });
+
+  describe('extractEnvVariable space trimming', () => {
+    beforeEach(() => {
+      process.env.HELLO = 'world';
+      process.env.USER = 'testuser';
+    });
+
+    it('should extract the value when string contains only an environment variable with surrounding whitespace', () => {
+      expect(extractEnvVariable('        ${HELLO}        ')).toBe('world');
+      expect(extractEnvVariable('  ${HELLO}  ')).toBe('world');
+      expect(extractEnvVariable('\t${HELLO}\n')).toBe('world');
+    });
+
+    it('should preserve content when variable is part of a larger string', () => {
+      expect(extractEnvVariable('Hello ${USER}!')).toBe('Hello testuser!');
+      expect(extractEnvVariable('  Hello ${USER}!  ')).toBe('Hello testuser!');
+    });
+
+    it('should not handle multiple variables', () => {
+      expect(extractEnvVariable('${HELLO} ${USER}')).toBe('${HELLO} ${USER}');
+      expect(extractEnvVariable('  ${HELLO}   ${USER}  ')).toBe('${HELLO}   ${USER}');
+    });
+
+    it('should handle undefined variables', () => {
+      expect(extractEnvVariable('        ${UNDEFINED_VAR}        ')).toBe('${UNDEFINED_VAR}');
+    });
+
+    it('should handle mixed content correctly', () => {
+      expect(extractEnvVariable('Welcome, ${USER}!\nYour message: ${HELLO}')).toBe(
+        'Welcome, testuser!\nYour message: world',
+      );
+    });
+  });
+});

packages/data-provider/src/azure.ts

@@ -6,8 +6,9 @@ import type {
   TValidatedAzureConfig,
   TAzureConfigValidationResult,
 } from '../src/config';
-import { errorsToString, extractEnvVariable, envVarRegex } from '../src/parsers';
+import { extractEnvVariable, envVarRegex } from '../src/utils';
 import { azureGroupConfigsSchema } from '../src/config';
+import { errorsToString } from '../src/parsers';
 
 export const deprecatedAzureVariables = [
   /* "related to" precedes description text */

packages/data-provider/src/index.ts

@@ -31,5 +31,6 @@ export { default as request } from './request';
 export { dataService };
 import * as dataService from './data-service';
 /* general helpers */
+export * from './utils';
 export * from './actions';
 export { default as createPayload } from './createPayload';

packages/data-provider/src/mcp.ts

@@ -1,4 +1,5 @@
 import { z } from 'zod';
+import { extractEnvVariable } from './utils';
 
 const BaseOptionsSchema = z.object({
   iconPath: z.string().optional(),
@@ -18,8 +19,22 @@ export const StdioOptionsSchema = BaseOptionsSchema.extend({
    * The environment to use when spawning the process.
    *
    * If not specified, the result of getDefaultEnvironment() will be used.
+   * Environment variables can be referenced using ${VAR_NAME} syntax.
    */
-  env: z.record(z.string(), z.string()).optional(),
+  env: z
+    .record(z.string(), z.string())
+    .optional()
+    .transform((env) => {
+      if (!env) {
+        return env;
+      }
+
+      const processedEnv: Record<string, string> = {};
+      for (const [key, value] of Object.entries(env)) {
+        processedEnv[key] = extractEnvVariable(value);
+      }
+      return processedEnv;
+    }),
   /**
    * How to handle stderr of the child process. This matches the semantics of Node's `child_process.spawn`.
    *

packages/data-provider/src/parsers.ts

@@ -19,6 +19,7 @@ import {
   compactAssistantSchema,
 } from './schemas';
 import { bedrockInputSchema } from './bedrock';
+import { extractEnvVariable } from './utils';
 import { alternateName } from './config';
 
 type EndpointSchema =
@@ -122,17 +123,6 @@ export function errorsToString(errors: ZodIssue[]) {
     .join(' ');
 }
 
-export const envVarRegex = /^\${(.+)}$/;
-
-/** Extracts the value of an environment variable from a string. */
-export function extractEnvVariable(value: string) {
-  const envVarMatch = value.match(envVarRegex);
-  if (envVarMatch) {
-    return process.env[envVarMatch[1]] || value;
-  }
-  return value;
-}
-
 /** Resolves header values to env variables if detected */
 export function resolveHeaders(headers: Record<string, string> | undefined) {
   const resolvedHeaders = { ...(headers ?? {}) };

packages/data-provider/src/utils.ts

@@ -0,0 +1,44 @@
+export const envVarRegex = /^\${(.+)}$/;
+
+/** Extracts the value of an environment variable from a string. */
+export function extractEnvVariable(value: string) {
+  if (!value) {
+    return value;
+  }
+
+  // Trim the input
+  const trimmed = value.trim();
+
+  // Special case: if it's just a single environment variable
+  const singleMatch = trimmed.match(envVarRegex);
+  if (singleMatch) {
+    const varName = singleMatch[1];
+    return process.env[varName] || trimmed;
+  }
+
+  // For multiple variables, process them using a regex loop
+  const regex = /\${([^}]+)}/g;
+  let result = trimmed;
+
+  // First collect all matches and their positions
+  const matches = [];
+  let match;
+  while ((match = regex.exec(trimmed)) !== null) {
+    matches.push({
+      fullMatch: match[0],
+      varName: match[1],
+      index: match.index,
+    });
+  }
+
+  // Process matches in reverse order to avoid position shifts
+  for (let i = matches.length - 1; i >= 0; i--) {
+    const { fullMatch, varName, index } = matches[i];
+    const envValue = process.env[varName] || fullMatch;
+
+    // Replace at exact position
+    result = result.substring(0, index) + envValue + result.substring(index + fullMatch.length);
+  }
+
+  return result;
+}