feat: agent changes

[LAVI9966]

No description provided.

[windsurf-bot]

💡 To request another review, post a new comment with "/windsurf-review".

[Copilot]

Pull request overview

This PR extends the "create agent" flow to leverage AI-generated configuration when a purpose field is supplied. It also enriches the AI prompt context with available API call tools and folder embed fields so the AI can produce a fully pre-configured agent.

Changes:

• AI-context enrichment: When purpose is provided, available API call functions (tools) and folder embed fields are now fetched and included as variables sent to the AI middleware.
• AI-driven agent creation: The AI response is used directly as the agent configuration (model, service, name, fall_back, gpt_memory, etc.) instead of extracting a small subset of fields as before.
• Minor cleanup: Removed an extraneous blank line in aiCall.utils.js.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
src/controllers/agentConfig.controller.js	Core logic change: fetches API call tools, passes enriched variables to AI, and uses the full AI response to populate the createAgent call when purpose is set.
src/services/utils/aiCall.utils.js	Trivial whitespace removal (blank line deleted).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Natwar589]

add validation in it we cannot believe on the agent reponse

[Husainbw786]

Code Review - PR #799

General Observations

This PR adds AI-powered agent configuration generation with new tools/fields context and restructures the configuration model with proper schemas.

---

Issues & Suggestions

1. Hardcoded version_id (Critical)

// aiCall.utils.js:35
requestBody.version_id = "699fd583a7ad6f86eb736a1e";

⚠️ This looks like a hardcoded version ID added for testing. Should this be configurable via environment variable or removed before merge?

---

2. Variable naming inconsistency (Minor)

// Line 97
const functions = await apiCallService.getAllApiCallsByOrgId(...);
// Line 100
tools: functions.map((tools) => { ... })

The variable is named functions but the callback parameter is tools. Suggest:

tools: functions.map((fn) => ({
  id: fn._id,
  description: fn.description
}))

---

3. Potential null reference (Bug Risk)

// Line 103-110
fields: folder_data
  ? folder_data?.config?.prompt?.embedFields
      ?.filter((field) => !field.hidden)
      ...

If folder_data is truthy but embedFields is undefined, the ?.filter() returns undefined, then the ?.reduce() fails. Consider:

fields: folder_data?.config?.prompt?.embedFields
  ?.filter((field) => !field.hidden)
  ?.reduce((acc, field) => {
    acc[field.name] = field.value || "";
    return acc;
  }, {}) || { role: "", goal: "", instruction: "" }

(Simplified - no need for outer ternary)

---

4. Typo fixed ✅

Good catch fixing "accroding" → "according" on line 114.

---

5. Repeated ternary logic (DRY opportunity)

Lines 221-228 repeat purpose ? agent_data?.X || default : default pattern multiple times. Consider:

const useAiData = purpose && agent_data;
const getValue = (aiVal, defaultVal) => useAiData ? (aiVal ?? defaultVal) : defaultVal;

const result = await ConfigurationServices.createAgent({
  ...(useAiData ? agent_data : {}),
  configuration: getValue(agent_data?.configuration, model_data),
  name: getValue(agent_data?.name, name),
  service: getValue(agent_data?.service, service),
  // etc.
});

---

6. Schema change impact

The configuration field type changed from Object to configurationSchema. Existing documents with non-conforming data may behave unexpectedly. Is there a migration needed?

---

7. Missing error handling

No error handling if apiCallService.getAllApiCallsByOrgId fails. Consider wrapping in try-catch or adding fallback:

const functions = await apiCallService.getAllApiCallsByOrgId(...).catch(() => []);

---

Summary

• 🔴 Remove/configure hardcoded version_id before merge
• 🟡 Consider null safety improvements
• 🟡 Schema migration for existing documents
• 🟢 Good refactoring of configuration logic

[Husainbw786]

Code Review for PR #799: feat: agent changes

🔴 Critical Issues

1. Security Risk: Unvalidated AI Response Spread into Database

Location: src/controllers/agentConfig.controller.js:229-230

const result = await ConfigurationServices.createAgent({
  ...(useAiData ? agent_data : {}),

Problem: The code directly spreads agent_data (raw AI response from callAiMiddleware) into the database object. This is a serious security vulnerability:

• An AI response could contain malicious or unexpected fields (e.g., org_id, user_id, _id, apikey, function_ids)
• These could override critical fields set later in the object
• The AI could return fields that shouldn't be user-controllable, leading to privilege escalation

Fix: Whitelist only the expected fields from the AI response:

const allowedAiFields = ['name', 'service', 'configuration', 'gpt_memory', 'fall_back'];
const safeAgentData = Object.keys(agent_data)
  .filter(key => allowedAiFields.includes(key))
  .reduce((obj, key) => ({ ...obj, [key]: agent_data[key] }), {});

const result = await ConfigurationServices.createAgent({
  ...(useAiData ? safeAgentData : {}),
  // ... rest of fields
});

---

2. Logic Bug: useAiData Truthy Check Incorrect

Location: src/controllers/agentConfig.controller.js:226

const useAiData = purpose && agent_data;

Problem: agent_data is initialized as {} (empty object), which is always truthy in JavaScript. So useAiData will be true whenever purpose exists, even if the AI call failed and agent_data is empty.

Fix: Check for actual AI response data:

const useAiData = purpose && Object.keys(agent_data).length > 0;

---

🟡 Moderate Issues

3. Missing Required Fields When Using AI Configuration

Location: src/controllers/agentConfig.controller.js:175-195

When purpose && agent_data?.configuration is true, the code uses model_data = agent_data.configuration directly. However, this may be missing required fields like:

• type (chat/completion)
• response_format
• is_rich_text
• prompt

This could create agents with incomplete configurations.

Fix: Merge with defaults:

if (purpose && agent_data?.configuration) {
  model_data = {
    type: type,
    response_format: { type: 'default', cred: {} },
    is_rich_text: false,
    prompt: prompt,
    ...agent_data.configuration
  };
}

---

4. Inconsistent Error Handling for Tools Fetch

Location: src/controllers/agentConfig.controller.js:93-99

The error is logged but silently swallowed. While this prevents the entire agent creation from failing, consider:

• Logging the error level (warn/error) consistently
• Perhaps including in the response that tools couldn't be loaded
• The purpose AI generation will proceed without tools, which may give suboptimal results

---

🟢 Minor Issues / Suggestions

5. Typo Fix in constant.js ✅

Good catch fixing "shcmea accroding" → "schema according". This improves AI prompt quality.

6. Rename Ambiguity

The variable res_data (line 107) shadows the common pattern of res for Express response. Consider ai_response or generatedConfig for clarity.

7. Optional: Add JSDoc Comments

The aiVal helper function is useful but could benefit from documentation:

/**
 * Returns AI value if AI data is being used and field exists, otherwise fallback
 * @param {*} aiField - Value from AI response
 * @param {*} fallback - Default value to use
 */
const aiVal = (aiField, fallback) => ...

---

Summary

Severity	Issue	Action
🔴 Critical	Unvalidated AI response spread	Whitelist allowed fields
🔴 Critical	Truthy check bug	Check Object.keys length
🟡 Moderate	Missing config fields	Merge with defaults
🟡 Moderate	Silent error swallow	Consider logging strategy
🟢 Good	Typo fix	✅ Approved

Please address the critical security and logic bugs before merging.