Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR discourse#36195

Type: Corrupted (contains bugs)

Original PR Title: DEV: Make persona image tools platform agnostic
Original PR Description: ## 🔍 Overview
This update updates existing persona tools: Image, CreateImage, and EditImage to make use of the new AI image generation tooling added in discourse@ce2e278 . It also updates the image generation tool scripts to support editing images on providers that support it.

📹 Preview

preview-gemini.mov

Original PR URL: discourse#36195


PR Type

Enhancement, Bug fix


Description

  • Migrate image generation tools to platform-agnostic custom tool system

  • Remove deprecated OpenAI and Stability image generation code

  • Add FLUX 2 Pro support via Black Forest Labs API

  • Update image editing to support multiple providers

  • Deprecate old image generation settings and personas


Diagram Walkthrough

flowchart LR
  A["Old Image Generators<br/>OpenAI/Stability"] -->|Deprecated| B["Custom Tool System<br/>AiTool"]
  B -->|Supports| C["FLUX 1.1 Pro<br/>Together.ai"]
  B -->|Supports| D["FLUX 2 Pro<br/>Black Forest Labs"]
  B -->|Supports| E["Gemini 2.5<br/>Google"]
  B -->|Supports| F["OpenAI GPT<br/>Image 1"]
  C -->|Generation/Edit| G["Image Tools<br/>Create/Edit/Image"]
  D -->|Generation/Edit| G
  E -->|Generation/Edit| G
  F -->|Generation/Edit| G
Loading

File Walkthrough

Relevant files
Enhancement
14 files
ai_tool.rb
Add FLUX 2 Pro preset and update FLUX naming                         
+29/-3   
artist.rb
Update Artist persona for custom image tools                         
+4/-3     
designer.rb
Update Designer persona for custom image tools                     
+7/-1     
general.rb
Conditionally include image tools based on configuration 
+6/-2     
persona.rb
Remove DallE3 from system personas and update tool availability
+3/-4     
create_image.rb
Refactor to use custom image generation tools                       
+73/-20 
edit_image.rb
Refactor to use custom tools with permission checks           
+87/-19 
image.rb
Refactor to delegate to custom image generation tools       
+87/-57 
tool.rb
Add method to retrieve available custom image tools           
+7/-0     
flux_2_bfl.js
Add FLUX 2 Pro image generation and editing script             
+130/-0 
flux_together.js
Add FLUX 1.1 Pro generation and editing script                     
+133/-0 
gemini.js
Add image editing support to Gemini script                             
+66/-1   
openai.js
Add image editing support to OpenAI script                             
+102/-1 
server.en.yml
Update FLUX naming and add FLUX 2 Pro label                           
+3/-1     
Bug fix
5 files
open_ai_image_generator.rb
Remove deprecated OpenAI image generator class                     
+0/-469 
stability_generator.rb
Remove deprecated Stability image generator class               
+0/-160 
dall_e_3.rb
Remove deprecated DALL-E 3 persona class                                 
+0/-37   
dall_e.rb
Remove deprecated DALL-E tool class                                           
+0/-97   
flux.js
Remove old FLUX script file                                                           
+0/-41   
Tests
7 files
playground_spec.rb
Remove deprecated DALL-E bot test cases                                   
+0/-63   
persona_spec.rb
Update persona tests for custom tool system                           
+41/-35 
create_image_spec.rb
Rewrite tests to use custom image generation tools             
+78/-109
dall_e_spec.rb
Remove deprecated DALL-E tool test file                                   
+0/-97   
edit_image_spec.rb
Rewrite tests for custom image editing tools                         
+145/-66
image_spec.rb
Rewrite tests to use custom image generation tools             
+51/-23 
stability_generator_spec.rb
Remove deprecated Stability generator test file                   
+0/-81   
Documentation
1 files
preamble.js
Document upload.getBase64 for image editing support           
+15/-0   
Configuration changes
1 files
settings.yml
Deprecate old image generation settings                                   
+24/-28 

keegangeorge and others added 5 commits November 24, 2025 11:40
## 🔍 Overview
This update updates existing persona tools: `Image`, `CreateImage`, and `EditImage` to make use of the new AI image generation tooling added in discourse@ce2e278 . It also updates the image generation tool scripts to support editing images on providers that support it.

Now that the persona tools are platform agnostic, the update also deprecates any irrelevant OpenAI or Stability Generation related code.

## 📹 Preview
@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Hardcoded API key risk

Description: The script hardcodes an API key placeholder (const apiKey = "YOUR_BFL_API_KEY";) and
performs external API calls from a tool script; if admins paste real secrets into shipped
scripts, they could be committed or exposed to non-admins through logs, UIs, or
exports—secrets must be stored in server-side settings and injected securely, not embedded
in JS.
flux_2_bfl.js [1-20]

Referred Code
/* eslint-disable no-undef, no-unused-vars */
const apiKey = "YOUR_BFL_API_KEY";

function invoke(params) {
  const prompt = params.prompt;
  const imageUrls = params.image_urls || [];

  let seed = parseInt(params.seed, 10);
  if (!(seed > 0)) {
    seed = Math.floor(Math.random() * 2147483647) + 1;
  }

  if (imageUrls.length > 0) {
    return performEdit(prompt, imageUrls, seed);
  } else {
    return performGeneration(prompt, seed);
  }
}

function performGeneration(prompt, seed) {
Hardcoded API key risk

Description: The tool script uses a hardcoded Together API key placeholder (const apiKey =
"YOUR_KEY";); embedding real keys in these client-executed or admin-editable scripts risks
key leakage—move credentials to server-side settings and reference them indirectly.
flux_together.js [1-20]

Referred Code
/* eslint-disable no-undef, no-unused-vars */
const apiKey = "YOUR_KEY";
const model = "black-forest-labs/FLUX.1.1-pro";

function invoke(params) {
  const prompt = params.prompt;
  const imageUrls = params.image_urls || [];

  // Determine mode: edit if image_urls provided, otherwise generate
  const isEditMode = imageUrls.length > 0;

  let seed = parseInt(params.seed, 10);
  if (!(seed > 0)) {
    seed = Math.floor(Math.random() * 1000000) + 1;
  }

  if (isEditMode) {
    return performEdit(prompt, imageUrls, seed);
  } else {
    return performGeneration(prompt, seed);
Hardcoded API key risk

Description: The OpenAI preset defines const apiKey = "YOUR_OPENAI_API_KEY";; encouraging or enabling
inline keys in scripts can lead to secret exposure—use secure server-side storage
(SiteSetting) and avoid embedding secrets into tool scripts.
openai.js [1-20]

Referred Code
/* eslint-disable no-undef, no-unused-vars */
const apiKey = "YOUR_OPENAI_API_KEY";

function invoke(params) {
  const prompt = params.prompt;
  const size = params.size || "1024x1024";
  const imageUrls = params.image_urls || [];

  // Determine mode: edit if image_urls provided, otherwise generate
  const isEditMode = imageUrls.length > 0;

  if (isEditMode) {
    return performEdit(prompt, size, imageUrls);
  } else {
    return performGeneration(prompt, size);
  }
}

function performGeneration(prompt, size) {
  const body = {
Hardcoded API key risk

Description: The Gemini preset contains const apiKey = "YOUR_GOOGLE_API_KEY";; embedding API keys in JS
tool scripts is insecure and may leak through logs or exports—prefer secure backend
configuration and injection over inline constants.
gemini.js [1-18]

Referred Code
/* eslint-disable no-undef, no-unused-vars */
const apiKey = "YOUR_GOOGLE_API_KEY";

function invoke(params) {
  const prompt = params.prompt;
  const imageUrls = params.image_urls || [];

  // Determine mode: edit if image_urls provided, otherwise generate
  const isEditMode = imageUrls.length > 0;

  if (isEditMode) {
    return performEdit(prompt, imageUrls);
  } else {
    return performGeneration(prompt);
  }
}

function performGeneration(prompt) {
Incomplete access control

Description: The code validates access to uploads only when upload.access_control_post_id is present;
uploads without this flag (e.g., from secure but not AC-post-bound contexts) may bypass
permission checks—ensure all provided uploads are visible to context.user (e.g., via
guardian.can_see_upload?) to prevent editing private assets by referencing short URLs.
edit_image.rb [63-83]

Referred Code
guardian = Guardian.new(context.user)
uploads = Upload.where(sha1: sha1s)

uploads.each do |upload|
  # Check if upload has access control
  if upload.access_control_post_id.present?
    post = Post.find_by(id: upload.access_control_post_id)
    if !guardian.can_see?(post)
      @error = true
      return(
        {
          prompt: prompt,
          error:
            "Access denied: You don't have permission to edit one or more of the provided images",
        }
      )
    end
  end
end

# Find available custom image generation tools
Unvalidated upload reference

Description: The implementation trusts custom_raw from custom tool scripts and parses the first
upload:// URL via regex, implicitly allowing any tool script to reference arbitrary upload
short URLs; without verifying ownership/visibility of the resolved Upload, a malicious
tool could embed private uploads—validate that resolved uploads are visible to
context.user before including them.
create_image.rb [74-91]

Referred Code
if tool_instance.custom_raw.present?
  # Parse the upload short_url from the markdown
  upload_match = tool_instance.custom_raw.match(%r{!\[.*?\]\((upload://[^)]+)\)})
  if upload_match
    short_url = upload_match[1]
    sha1 = Upload.sha1_from_short_url(short_url)
    upload = Upload.find_by(sha1: sha1) if sha1
    uploads << { prompt: prompt, upload: upload, url: short_url } if upload
  else
    # Tool returned custom_raw but not in expected format
    Rails.logger.error(
      "CreateImage: Tool #{tool_class.name} returned custom_raw in unexpected format. " \
        "Expected markdown with upload:// URL. " \
        "custom_raw preview: #{tool_instance.custom_raw.truncate(200)}",
    )
    errors << "Tool returned invalid image format"
  end
else
Unvalidated upload reference

Description: Similar to CreateImage, this tool trusts custom_raw from custom tools and resolves
upload:// to an Upload without permission checks; a malicious or misconfigured tool could
leak private uploads—verify guardian access for resolved uploads before using them.
image.rb [107-121]

Referred Code
if tool_instance.custom_raw.present?
  # Parse the upload short_url from the markdown
  upload_match = tool_instance.custom_raw.match(%r{!\[.*?\]\((upload://[^)]+)\)})
  if upload_match
    short_url = upload_match[1]
    sha1 = Upload.sha1_from_short_url(short_url)
    upload = Upload.find_by(sha1: sha1) if sha1
    if upload
      uploads << {
        prompt: prompt,
        upload: upload,
        seed: nil, # Custom tools don't provide seeds
      }
    end
  end
Potential data exfiltration

Description: The new upload.getBase64(uploadIdOrShortUrl) surface allows tool scripts to fetch raw
image data for any upload ID or short URL; unless the backend enforces permission checks
on this helper, tools could exfiltrate private uploads—ensure getBase64 enforces
context.user visibility on the referenced upload.
preamble.js [86-100]

Referred Code
*    upload.getBase64(uploadIdOrShortUrl, maxPixels): Fetches the base64-encoded content of an existing upload.
*    Parameters:
*      uploadIdOrShortUrl (number | string): Either an upload ID (number) or short URL (string, eg "upload://abc123").
*      maxPixels (number, optional): Maximum pixel count for automatic resizing (default: 10,000,000).
*    Returns: string (base64-encoded image data) or null if upload not found.
*    Use case: Image editing tools can fetch existing uploads to send to external APIs.
*
*    Note for Image Editing:
*    To implement image editing in a tool:
*    1. Accept an `image_urls` parameter (array of short URLs like ["upload://abc123"]).
*    2. Use upload.getBase64() to fetch the base64 data for each image.
*    3. Send the base64 data to your image editing API (e.g., OpenAI's /v1/images/edits endpoint).
*    4. Create a new upload with the edited image using upload.create().
*    5. Use chain.setCustomRaw() to display the edited image.
* 5. chain
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Leaky Error Message: User-facing errors return raw exception messages (e.message), which can expose internal
details contrary to secure error handling guidance.

Referred Code
rescue => e
  @error = true
  Rails.logger.error(
    "EditImage: Failed to edit image. " \
      "Tool: #{tool_class.name}, Error: #{e.class.name} - #{e.message}. " \
      "Prompt: #{prompt.truncate(50)}, Image URLs: #{image_urls.join(", ")}",
  )
  { prompt: prompt, error: e.message }
end

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Audit Logs: New image edit flows validate permissions but do not log who attempted edits or failures,
reducing traceability of critical actions.

Referred Code
guardian = Guardian.new(context.user)
uploads = Upload.where(sha1: sha1s)

uploads.each do |upload|
  # Check if upload has access control
  if upload.access_control_post_id.present?
    post = Post.find_by(id: upload.access_control_post_id)
    if !guardian.can_see?(post)
      @error = true
      return(
        {
          prompt: prompt,
          error:
            "Access denied: You don't have permission to edit one or more of the provided images",
        }
      )
    end
  end
end

# Find available custom image generation tools


 ... (clipped 63 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Generic Error Propagation: Errors from underlying tools are surfaced directly via e.message to users without
normalization, and multiple failures only return the first error, which may lack
actionable context.

Referred Code
  rescue => e
    Rails.logger.error(
      "CreateImage: Failed to generate image for prompt '#{prompt.truncate(50)}'. " \
        "Tool: #{tool_class.name}, Error: #{e.class.name} - #{e.message}",
    )
    errors << e.message
  end
end

if uploads.empty?
  @error = true
  return(
    {
      prompts: max_prompts,
      error:
        "Failed to generate images. #{errors.first || "Please check your image generation tool configuration."}",
    }
  )

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Log Data Sensitivity: Logs include prompt previews and raw tool output previews which may contain user-provided
sensitive data; confirm redaction policies before logging.

Referred Code
      Rails.logger.error(
        "CreateImage: Tool #{tool_class.name} returned custom_raw in unexpected format. " \
          "Expected markdown with upload:// URL. " \
          "custom_raw preview: #{tool_instance.custom_raw.truncate(200)}",
      )
      errors << "Tool returned invalid image format"
    end
  else
    # Tool returned no output
    Rails.logger.warn(
      "CreateImage: Tool #{tool_class.name} returned no custom_raw output for prompt: #{prompt.truncate(50)}",
    )
    errors << "Tool returned no output"
  end
rescue => e
  Rails.logger.error(
    "CreateImage: Failed to generate image for prompt '#{prompt.truncate(50)}'. " \
      "Tool: #{tool_class.name}, Error: #{e.class.name} - #{e.message}",
  )
  errors << e.message

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Hardcoded API Key: The preset script includes a hardcoded API key placeholder and constructs external
requests; ensure keys are injected securely and validate/limit external calls.

Referred Code
/* eslint-disable no-undef, no-unused-vars */
const apiKey = "YOUR_BFL_API_KEY";

function invoke(params) {
  const prompt = params.prompt;
  const imageUrls = params.image_urls || [];

  let seed = parseInt(params.seed, 10);
  if (!(seed > 0)) {
    seed = Math.floor(Math.random() * 2147483647) + 1;
  }

  if (imageUrls.length > 0) {
    return performEdit(prompt, imageUrls, seed);
  } else {
    return performGeneration(prompt, seed);
  }
}

function performGeneration(prompt, seed) {

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Refactor tool dispatching logic

The image generation tools (Image, CreateImage, EditImage) duplicate logic for
selecting an image provider and always pick the first one available. This should
be centralized to allow users or the LLM to choose from multiple configured
providers.

Examples:

plugins/discourse-ai/lib/personas/tools/create_image.rb [52]
          tool_class = custom_tools.first
plugins/discourse-ai/lib/personas/tools/edit_image.rb [99]
          tool_class = custom_tools.first

Solution Walkthrough:

Before:

// In CreateImage.rb, EditImage.rb, and Image.rb

def invoke
  // ...
  custom_tools = self.class.available_custom_image_tools

  if custom_tools.empty?
    return { error: "No image generation tools configured." }
  end

  // Always uses the first available tool, with no way to choose.
  tool_class = custom_tools.first

  // ... logic to invoke tool_class ...
end

After:

# In a new helper or within the tools themselves
def find_image_tool(requested_tool_name: nil)
  custom_tools = self.class.available_custom_image_tools
  return nil if custom_tools.empty?

  if requested_tool_name
    custom_tools.find { |t| t.name == requested_tool_name } || custom_tools.first
  else
    custom_tools.first
  end
end

# In CreateImage.rb (and similarly in EditImage/Image)
# Update signature to accept an optional tool_name parameter

def invoke
  tool_class = find_image_tool(requested_tool_name: parameters[:tool_name])
  # ... logic to invoke tool_class ...
end
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a significant design flaw where the implementation always defaults to the first available image tool, undermining the PR's goal of being platform-agnostic and preventing users from selecting between multiple configured providers.

Medium
Possible issue
Prevent nil error in permission check

Add a nil check for the post variable before calling guardian.can_see?(post) to
prevent a potential NoMethodError if the post is not found.

plugins/discourse-ai/lib/personas/tools/edit_image.rb [69-80]

 def invoke
   yield(prompt)
 
   if image_urls.blank?
     @error = true
     return { prompt: prompt, error: "No valid images provided" }
   end
 
   # Validate that the image URLs exist
   sha1s = image_urls.map { |url| Upload.sha1_from_short_url(url) }.compact
   if sha1s.empty?
     @error = true
     return { prompt: prompt, error: "No valid image URLs provided" }
   end
 
   # Check permissions - use context.user (the human) not bot_user
   guardian = Guardian.new(context.user)
   uploads = Upload.where(sha1: sha1s)
 
   uploads.each do |upload|
     # Check if upload has access control
     if upload.access_control_post_id.present?
       post = Post.find_by(id: upload.access_control_post_id)
-      if !guardian.can_see?(post)
+      if !post || !guardian.can_see?(post)
         @error = true
         return(
           {
             prompt: prompt,
             error:
               "Access denied: You don't have permission to edit one or more of the provided images",
           }
         )
       end
     end
   end
   ...

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential NoMethodError if Post.find_by returns nil and guardian.can_see? is called on it, which would cause an unhandled exception. Adding a !post check is a valid and important fix for this edge case.

Medium
Add error handling for JSON parsing

Wrap the JSON.parse(submitResult.body) call in a try...catch block to handle
non-JSON API responses gracefully and prevent script crashes.

plugins/discourse-ai/lib/ai_tool_scripts/presets/image_generation/flux_2_bfl.js [65-69]

 function submitAndPoll(body, prompt, seed) {
   // Submit request
   const submitResult = http.post("https://api.bfl.ai/v1/flux-2-pro", {
     headers: {
       "x-key": apiKey,
       "Content-Type": "application/json",
     },
     body: JSON.stringify(body),
   });
 
-  const submitData = JSON.parse(submitResult.body);
+  let submitData;
+  try {
+    submitData = JSON.parse(submitResult.body);
+  } catch (e) {
+    return {
+      error: "Failed to parse BFL API submission response.",
+      body_preview: submitResult.body.substring(0, 500),
+    };
+  }
 
   if (submitData.error) {
     return { error: `BFL API Error: ${submitData.error}` };
   }
   ...

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that JSON.parse can fail if the API returns a non-JSON response, leading to a script crash. Wrapping it in a try...catch block is a good practice for robust error handling.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants