core(clean): clean code base dto/dc/naming and little improvement #220
Conversation
WalkthroughRemoved Starknet integration and many legacy agent/operator/MCP components; consolidated type imports (.types.js → .type.js); switched default models to Google Gemini and added GEMINI/SMITHERY env/config; reorganized prompts and DTOs; introduced MCP server discovery tool and Gemini-only model initialization and streaming execution flow. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant Service
participant Agent
participant Executor
participant Gemini as GeminiModel
Client->>Gateway: send AgentRequestDTO (agent_id, content)
Gateway->>Service: onAgentRequest(agentId, content)
Service->>Agent: executeAsyncGenerator(content)
rect rgb(220,240,250)
Note over Agent,Executor: Initialization
Agent->>Agent: init()
Agent->>Executor: createAgentReactExecutor()
end
rect rgb(240,250,240)
Note over Executor,Gemini: Streaming loop
Executor->>Gemini: request tokens (model, temp)
Gemini-->>Executor: token chunk
Executor-->>Agent: ChunkOutput
Agent-->>Service: yield ChunkOutput (async generator)
Service-->>Gateway: stream ChunkOutput to Client
end
rect rgb(250,240,240)
Note over Agent: Teardown
Agent->>Agent: dispose(), cleanup resources (RagAgent, checkpointer)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0xhijo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
packages/core/src/config/database.config.ts (1)
27-39: Add environment variable validation per PR objectives.The
initialize()method reads environment variables without validating their existence or format. The PR objectives explicitly state: "Add improved environment security: checks and more explicit errors when required env fields are missing." Currently, missing env vars silently becomeundefined, and theas stringtype assertions mask this. TheparseInton line 37 can also fail ifPOSTGRES_PORTis unset or invalid.Add validation to ensure all required environment variables are present and properly formatted:
public initialize(): void { if (this.databaseCredentials) { return; // Already initialized } + const requiredEnvVars = { + POSTGRES_DB: process.env.POSTGRES_DB, + POSTGRES_HOST: process.env.POSTGRES_HOST, + POSTGRES_USER: process.env.POSTGRES_USER, + POSTGRES_PASSWORD: process.env.POSTGRES_PASSWORD, + POSTGRES_PORT: process.env.POSTGRES_PORT, + }; + + const missingVars = Object.entries(requiredEnvVars) + .filter(([_, value]) => !value) + .map(([key]) => key); + + if (missingVars.length > 0) { + throw new Error( + `Missing required environment variables: ${missingVars.join(', ')}` + ); + } + + let port: number; + try { + port = parseInt(requiredEnvVars.POSTGRES_PORT!, 10); + if (isNaN(port)) { + throw new Error('Port must be a valid number'); + } + } catch (error) { + throw new Error( + `Invalid POSTGRES_PORT: ${requiredEnvVars.POSTGRES_PORT}. Must be a valid number.` + ); + } this.databaseCredentials = { - database: process.env.POSTGRES_DB as string, - host: process.env.POSTGRES_HOST as string, - user: process.env.POSTGRES_USER as string, - password: process.env.POSTGRES_PASSWORD as string, - port: parseInt(process.env.POSTGRES_PORT as string), + database: requiredEnvVars.POSTGRES_DB!, + host: requiredEnvVars.POSTGRES_HOST!, + user: requiredEnvVars.POSTGRES_USER!, + password: requiredEnvVars.POSTGRES_PASSWORD!, + port, }; }packages/agent/src/tools/tools.ts (1)
73-74: Remove commented-out code.These commented lines should be removed as part of this cleaning PR. Dead code contradicts the PR objective to "clean code base" and "remove dead code."
Apply this diff:
- // Register memory tools - // const memoryRegistry = new MemoryToolRegistry(agentConfig); - // toolsList.push(...memoryRegistry.getTools()); -tests/src/file-test/file-upload.ts (1)
172-176: Guard cleanup to avoid invalid delete when creation fails or agent lookup missesIf create fails,
agentIdremains empty, but the finally block still executes and attempts deletion.Apply:
- } finally { - await testRunner.runTest('Cleanup - Delete Test Agent', () => - testRunner.client.deleteAgent(agentId) - ); - } + } finally { + if (agentId) { + await testRunner.runTest('Cleanup - Delete Test Agent', () => + testRunner.client.deleteAgent(agentId) + ); + } else { + console.log(chalk.yellow('Cleanup skipped: no agentId available')); + } + }packages/server/src/agents.storage.ts (1)
400-431: Remove unusedagentConfigResolverfunction (lines 400-431).The
agentConfigResolverfunction is defined but never used anywhere in the codebase. The ripgrep search confirms no actual references to this variable exist—only the text "agentConfigResolver:" appearing in logger strings. This is dead code from a previous refactoring that should be deleted.packages/core/src/services/agent-validation.service.ts (1)
307-311: Bug: truthy check skips validation for 0; use explicit undefined/null checkUsing if (model.max_tokens) bypasses validation when max_tokens is 0. Prefer explicit checks.
- if (model.max_tokens) { + if (model.max_tokens !== undefined && model.max_tokens !== null) { if (model.max_tokens > maxTokens) { throw new Error(`Max tokens too high. Maximum value: ${maxTokens}`); } }packages/core/src/common/server/dto/agent/config.dto.ts (1)
60-68: class-validator misuse: @matches on a number will always fail@matches applies to strings. On a numeric property it produces a validation error even for valid numbers. Use @IsNumber with Min/Max.
- @Min(0) - @Max(2) - @Matches(/^-?\d+(\.\d+)?$/, { message: 'Temperature must be a number' }) - temperature: number; + @IsNumber({ allowNaN: false, allowInfinity: false }, { message: 'Temperature must be a number' }) + @Min(0) + @Max(2) + temperature: number;packages/server/src/services/agent.service.ts (2)
69-96: Final result detection misses valid final chunks when event != ON_CHAIN_ENDStreams can signal final via metadata.final regardless of event. Current check only returns on ON_CHAIN_END, causing "No final chunk received" even when final was yielded (e.g., ON_CHAT_MODEL_END).
- if ( - chunk.event === EventType.ON_CHAT_MODEL_END || - chunk.event === EventType.ON_CHAIN_END - ) { + if ( + chunk.event === EventType.ON_CHAT_MODEL_END || + chunk.event === EventType.ON_CHAIN_END + ) { const messageId = await message.insert_message( agent.getAgentConfig().id, userId, chunk ); this.logger.debug( `Inserted message with ID: ${messageId.toLocaleString()}` ); - if ( - chunk.event === EventType.ON_CHAIN_END && - chunk.metadata.final === true - ) { + if (chunk.metadata?.final === true) { result = chunk; return { status: 'success', data: result, }; } }Also applies to: 85-93, 105-111
49-57: Fix agent implementations to use.contentinstead of.requestfrom MessageRequestThe migration from
.requestto.contentis incomplete. Whileagent.service.tscorrectly accessesuserRequest.content, the agent implementations still try to access.requeston theMessageRequesttype parameter, which only defines acontentfield:
packages/agent/src/agents/core/snakAgent.ts: lines 297, 305, 311, 334, 379, 410 accessrequest.requestpackages/agent/src/agents/core/supervisorAgent.ts: lines 191–192, 219, 254 accessuserRequest.requestThese accesses will evaluate to
undefinedat runtime, causing errors when the string content is needed (e.g., when creating messages or parsing user input).packages/agent/src/agents/core/snakAgent.ts (1)
357-361: Bug: step_id assignment gated by task_id presenceCurrent condition checks additional_kwargs.task_id instead of step_id; step_id can exist independently.
- currentStepId = - chunk.event === 'on_chat_model_end' && - chunk.data?.output?.additional_kwargs?.task_id - ? chunk.data.output.additional_kwargs.step_id - : null; + currentStepId = + chunk.event === 'on_chat_model_end' && + chunk.data?.output?.additional_kwargs?.step_id + ? chunk.data.output.additional_kwargs.step_id + : null;packages/server/src/controllers/gateway.controller.ts (2)
56-58: Avoid logging full user payload; log minimal metadata.Reduce PII/secret leakage risk by not JSON-stringifying the entire request.
-logger.debug(`handleUserRequest: ${JSON.stringify(userRequest)}`); +logger.debug({ + msg: 'handleUserRequest', + agent_id: userRequest?.request?.agent_id, + thread_id: userRequest?.request?.thread_id, +});
85-87: Potential runtime error: toLocaleString on non-number IDs.Use String(messageId) to log safely.
- logger.info( - `Inserted message with ID: ${messageId.toLocaleString()}` - ); + logger.info(`Inserted message with ID: ${String(messageId)}`);
🧹 Nitpick comments (24)
packages/agent/src/shared/prompts/agents/snak/core/index.ts (1)
5-8: Standardize filename convention for consistency.The imported files use inconsistent naming: some use singular
prompt.js(lines 5, 7) while others use pluralprompts.js(lines 6, 8). Since this PR focuses on cleaning the codebase and improving naming, consider standardizing on one convention.Suggested approach: standardize on singular
prompt.jsfor consistency:export * from './task-executor.prompt.js'; -export * from './task-manager.prompts.js'; +export * from './task-manager.prompt.js'; export * from './task-memory-manager.prompt.js'; -export * from './task-verifier.prompts.js'; +export * from './task-verifier.prompt.js';This requires renaming the source files accordingly.
packages/core/src/common/constant/default-agent.constant.ts (1)
1-5: Path migration LGTM; prefer type-only imports for pure typesAgentConfig and ModelConfig are types only; importing them as types avoids a runtime ESM dependency on agent.interface.js. Keep MemoryStrategy as a value import.
Proposed tweak:
-import { - AgentConfig, - MemoryStrategy, - ModelConfig, -} from '@common/agent/interfaces/agent.interface.js'; +import type { AgentConfig, ModelConfig } from '@common/agent/interfaces/agent.interface.js'; +import { MemoryStrategy } from '@common/agent/interfaces/agent.interface.js';packages/agent/src/agents/core/supervisorAgent.ts (1)
7-7: Use type-only imports to avoid unnecessary runtime dependenciesChunkOutput, ChunkOutputMetadata, GraphErrorType, and UserRequest are types; importing as types reduces runtime module coupling.
Suggested diff:
-import { - ChunkOutput, - ChunkOutputMetadata, -} from '../../shared/types/streaming.type.js'; +import type { ChunkOutput, ChunkOutputMetadata } from '../../shared/types/streaming.type.js'; @@ -import { GraphErrorType, UserRequest } from '@stypes/graph.type.js'; +import type { GraphErrorType, UserRequest } from '@stypes/graph.type.js';Also applies to: 15-15
packages/agent/src/agents/core/baseAgent.ts (2)
2-2: Type-only import for ChunkOutputThis is a type; prefer
import typeto avoid runtime dependency.-import { ChunkOutput } from '../../shared/types/streaming.type.js'; +import type { ChunkOutput } from '../../shared/types/streaming.type.js';
10-10: Confirm removal of IAgent does not break consumersDropping
implements IAgentremoves a compile-time contract. Ensure external consumers and tests depend on BaseAgent’s concrete API, not the interface. Consider keeping a minimal interface in shared/types for stability, or add doc comments indicating the expectedexecute/initshape.tests/src/file-test/file-upload.ts (1)
26-26: Stabilize: capture created agent ID from the create responseInstead of listing agents and finding by name, grab the ID directly from the createAgent response to avoid race/flakiness and reduce API calls.
Example (adjust to actual response shape):
- const createResult = await testRunner.runTest('Create Agent for File Testing', () => - testRunner.client.createAgent({ + const createResult = await testRunner.runTest('Create Agent for File Testing', () => + testRunner.client.createAgent({ agent: { name: 'File Test Agent', group: 'test', description: 'Agent for testing file upload functionality', ... } }) ); @@ - let agentId = ''; + let agentId = ''; try { - const agentsResult = await testRunner.runTest('Get Agents List', () => - testRunner.client.getAgents() - ); - ... - const fileTestAgent = agentsList.find((agent: any) => agent.name === 'File Test Agent'); - ... - agentId = fileTestAgent.id; + // Prefer the ID returned by createAgent, if present. + if (createResult.success && (createResult as any).response?.agent?.id) { + agentId = (createResult as any).response.agent.id; + } else { + // Fallback: list agents if the API doesn’t return the new id. + const agentsResult = await testRunner.runTest('Get Agents List', () => testRunner.client.getAgents()); + if (!agentsResult.success || !agentsResult.response) { ... } + const agentsList = (agentsResult.response as any[]) || []; + const fileTestAgent = agentsList.find((agent: any) => agent.name === 'File Test Agent'); + if (!fileTestAgent) { ... } + agentId = fileTestAgent.id; + }packages/core/src/common/server/dto/agent/message.dto.ts (1)
85-85: Follow PascalCase convention for class name.The class name
getMessagesFromAgentsDTOshould follow PascalCase convention:GetMessagesFromAgentsDTO.Apply this diff:
-export class getMessagesFromAgentsDTO { +export class GetMessagesFromAgentsDTO {packages/core/src/services/agent-validation.service.ts (1)
195-206: Numeric validation should assert type and finiteness, not only NaN and boundsRelying on Number.isNaN for possibly string inputs can pass invalid types; add typeof === 'number' and Number.isFinite checks; enforce lower bounds where applicable (e.g., temperature >= 0).
- if (Number.isNaN(graph.max_steps)) { + if (typeof graph.max_steps !== 'number' || !Number.isFinite(graph.max_steps)) { throw new Error('Max steps must be a valid number'); } ... - if (Number.isNaN(graph.max_iterations)) { + if (typeof graph.max_iterations !== 'number' || !Number.isFinite(graph.max_iterations)) { throw new Error('Max iterations must be a valid number'); } ... - if (Number.isNaN(graph.max_retries)) { + if (typeof graph.max_retries !== 'number' || !Number.isFinite(graph.max_retries)) { throw new Error('Max retries must be a valid number'); } ... - if (Number.isNaN(graph.execution_timeout_ms)) { + if (typeof graph.execution_timeout_ms !== 'number' || !Number.isFinite(graph.execution_timeout_ms)) { throw new Error('Execution timeout must be a valid number'); } ... - if (Number.isNaN(graph.max_token_usage)) { + if (typeof graph.max_token_usage !== 'number' || !Number.isFinite(graph.max_token_usage)) { throw new Error('Max token usage must be a valid number'); } ... - if (model.temperature !== undefined) { - if (model.temperature > maxTemperature) { + if (model.temperature !== undefined && model.temperature !== null) { + if (typeof model.temperature !== 'number' || !Number.isFinite(model.temperature)) { + throw new Error('Temperature must be a valid number'); + } + if (model.temperature < 0) { + throw new Error('Temperature must be >= 0'); + } + if (model.temperature > maxTemperature) { throw new Error( `Temperature too high. Maximum value: ${maxTemperature}` ); } }Also applies to: 209-218, 221-228, 231-244, 246-255, 298-305
packages/server/src/services/agent.service.ts (3)
146-155: Logging consistency and PII minimizationLog content consistently as userRequest.content and avoid logging whole DTOs. Consider truncating content and including thread_id/agent_id instead.
- this.logger.debug({ - message: 'Processing agent request', - request: userRequest.content, - }); + this.logger.debug({ + message: 'Processing agent request', + requestPreview: userRequest.content?.slice(0, 200), + }); ... - this.logger.error('Error processing agent request', { + this.logger.error('Error processing agent request', { error: { message: error.message, name: error.name, stack: error.stack, }, - request: userRequest, + requestPreview: userRequest.content?.slice(0, 200), });Also applies to: 171-179, 54-57, 128-129
66-70: Safer async generator detectionUse a function check to avoid false positives.
- if (Symbol.asyncIterator in executeResult) { + if (executeResult && typeof executeResult[Symbol.asyncIterator] === 'function') {
210-221: Shadowed identifier 'agents' masks imported queriesLocal const agents shadows the imported agents query module. Rename to avoid confusion.
- const agents = await redisAgents.listAgentsByUser(userId); + const redisAgentList = await redisAgents.listAgentsByUser(userId); ... - const agentsWithoutUserId = agents.map((agent) => { + const agentsWithoutUserId = redisAgentList.map((agent) => {packages/agent/src/agents/operators/supervisor/tools/searchMcpServerTools.ts (2)
76-84: Add timeouts for external fetches to avoid hanging toolsNetwork calls should fail fast. Use AbortController with a timeout.
+ const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10000); const response = await fetch( `https://registry.smithery.ai/servers?${searchParams.toString()}`, { headers: { Authorization: `Bearer ${apiKey}`, Accept: 'application/json', }, + signal: controller.signal, } ); + clearTimeout(timeout); ... - const detailResponse = await fetch( + const detailController = new AbortController(); + const detailTimeout = setTimeout(() => detailController.abort(), 10000); + const detailResponse = await fetch( `https://registry.smithery.ai/servers/${encodeURIComponent(server.qualifiedName)}`, { headers: { Authorization: `Bearer ${apiKey}`, Accept: 'application/json', }, + signal: detailController.signal, } ); + clearTimeout(detailTimeout);Also applies to: 118-129
97-103: Remove bodyUsed checks and align fallback shapebodyUsed is unnecessary immediately after fetch; fallback returns configSchema but success path doesn’t — align shapes.
- if (response.bodyUsed) { - throw new Error( - 'Response body already consumed in main search request' - ); - } ... - if (detailResponse.bodyUsed) { - throw new Error( - `Response body already consumed for server ${server.qualifiedName}` - ); - } ... - return { - ...server, - connections: [], - tools: [], - configSchema: null, - }; + return { + ...server, + connections: [], + tools: [], + }; ... - return { + return { ...server, connections: [], tools: [],Also applies to: 143-149, 135-141, 193-204
packages/agent/src/agents/core/snakAgent.ts (5)
29-33: Outdated docstring mentions Starknet; update to reflect current scopeThe class no longer integrates Starknet. Update the comment to avoid confusion.
207-213: Content serialization via toLocaleString can mangle structured outputsLangChain content may be arrays/blocks. Prefer a safe string extractor.
- ? chunk.data.output.content.toLocaleString() + ? (Array.isArray(chunk.data.output.content) + ? chunk.data.output.content.map(String).join(' ') + : String(chunk.data.output.content))
368-374: Guard against missing tasks/interrupts when notifyingState shape may not always include tasks[0].interrupts[0].
- await notify.insertNotify( - this.agentConfig.user_id, - this.agentConfig.id, - stateSnapshot.tasks[0].interrupts[0].value - ); + const value = stateSnapshot?.tasks?.[0]?.interrupts?.[0]?.value; + if (value) { + await notify.insertNotify( + this.agentConfig.user_id, + this.agentConfig.id, + value + ); + } else { + logger.warn('[SnakAgent] interrupt value missing; skipping notify'); + }
421-427: Don’t swallow execution errors silentlyReturning from catch hides failures. Either rethrow or yield a final error chunk to inform callers.
- if (this.isTokenRelatedError(error)) { + if (this.isTokenRelatedError(error)) { logger.warn('[SnakAgent] Token limit exceeded'); - throw new Error( + throw new Error( 'The request could not be completed because it exceeded the token limit. Please try again with a shorter input or reduce the complexity of the task.' ); } } } catch (error: any) { - logger.error(`[SnakAgent] Execution failed: ${error}`); - return; + logger.error(`[SnakAgent] Execution failed: ${error}`); + throw error; }Also applies to: 429-432
102-108: TODO indicates RagAgent doesn’t need full agentConfig — tighten dependencyIf RagAgent can be initialized with a minimal shape, pass only what's required and remove the TODO.
packages/core/src/common/server/dto/agent/config.dto.ts (1)
29-36: Consolidate duplicate DTOs: four definitions serve two purposesFour DTO classes are defined but only serve two distinct purposes:
- Create/add agent:
AddAgentRequestDTOandAgentAddRequestDTOare identical—both acceptagent: AgentConfig.Input- Delete agents:
AgentsDeleteRequestDTOandAgentDeletesRequestDTOare identical—both acceptagent_id: string[]This splitting is currently reflected across controllers (gateway.controller.ts uses
AddAgentRequestDTO; agents.controller.ts usesAgentAddRequestDTOandAgentDeletesRequestDTO).Consolidate to single canonical names per operation and update the two controller files to use them. For backward compatibility during transition, export temporary type aliases from the old names and mark them deprecated.
packages/agent/src/index.ts (1)
30-31: Tidy orphan comment."Tool-related exports" is empty; replace with a short note or remove.
-// Tool-related exports - +// (tools exports removed; see CHANGELOG for migration)packages/server/src/controllers/gateway.controller.ts (2)
62-65: Simplify assignment.No need for a mutable variable here.
- let agent: BaseAgent | undefined; - - agent = await this.agentFactory.getAgentInstance( + const agent = await this.agentFactory.getAgentInstance( userRequest.request.agent_id, userId );
27-33: Make CORS origin configurable via env.Hardcoded localhost hinders staging/prod and risks misconfig. Drive from WS_CORS_ORIGIN.
@WebSocketGateway({ cors: { - origin: 'http://localhost:4000', + origin: process.env.WS_CORS_ORIGIN?.split(',') ?? 'http://localhost:4000', methods: ['GET', 'POST'], credentials: true, }, })packages/core/src/common/server/dto/agent/mcp.dto.ts (2)
111-121: Strengthen validation for mcpServers (nested object).Validate shape of each server config; also normalize args to string[].
-import { - IsNotEmpty, IsString, IsUUID, IsArray, ArrayNotEmpty, -} from 'class-validator'; +import { + IsNotEmpty, IsString, IsUUID, IsArray, ArrayNotEmpty, IsObject, ValidateNested, IsOptional, +} from 'class-validator'; +import { Type, Transform } from 'class-transformer'; + +class McpServerConfigDTO { + @IsNotEmpty() @IsString() command!: string; + @IsOptional() + @Transform(({ value }) => (value == null ? undefined : (Array.isArray(value) ? value : [String(value)]))) + @IsArray() @IsString({ each: true }) + args?: string[]; + @IsOptional() @IsObject() + env?: Record<string, string>; + // Allow extra keys but prefer to document them explicitly if known. +} export class AddMcpServerRequestDTO { @IsNotEmpty() @IsUUID() agent_id: string; @IsNotEmpty() - mcpServers: Record< - string, - { - command: string; - args?: string[] | string; - env?: Record<string, string>; - [key: string]: any; - } - >; + @IsObject() + @ValidateNested({ each: true }) + @Type(() => McpServerConfigDTO) + mcpServers: Record<string, McpServerConfigDTO>; }
9-16: Fix comment/intent mismatch.Docstrings for GetAgentMcpsRequestDTO and AgentMCPRequestDTO read ambiguously. Suggest:
- GetAgentMcpsRequestDTO: "Request to get all MCP servers for an agent".
- AgentMCPRequestDTO: "Request to target a single MCP server of an agent".
- * Request to get a specific agent's MCP config + * Request to get all MCP servers for an agent ... - * Request to get all MCP server of a specific agent + * Request to target a single MCP server of a specific agentAlso applies to: 18-29
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (91)
.env.example(1 hunks)config/guards/default.guards.json(1 hunks)package.json(0 hunks)packages/agent/src/__mocks__/@snakagent/core.ts(0 hunks)packages/agent/src/agents/core/baseAgent.ts(1 hunks)packages/agent/src/agents/core/snakAgent.ts(3 hunks)packages/agent/src/agents/core/supervisorAgent.ts(1 hunks)packages/agent/src/agents/graphs/__tests__/task-verifier-graph.test.ts(1 hunks)packages/agent/src/agents/graphs/core-graph/supervisor.graph.ts(2 hunks)packages/agent/src/agents/graphs/manager/memory/memory-db-manager.ts(1 hunks)packages/agent/src/agents/graphs/parser/memory/stm-parser.ts(1 hunks)packages/agent/src/agents/graphs/parser/tasks/tasks.parser.ts(1 hunks)packages/agent/src/agents/graphs/sub-graph/task-executor.graph.ts(2 hunks)packages/agent/src/agents/graphs/sub-graph/task-manager.graph.ts(1 hunks)packages/agent/src/agents/graphs/sub-graph/task-memory.graph.ts(2 hunks)packages/agent/src/agents/graphs/sub-graph/task-verifier.graph.ts(1 hunks)packages/agent/src/agents/graphs/tools/core.tools.ts(1 hunks)packages/agent/src/agents/graphs/tools/memory.tool.ts(1 hunks)packages/agent/src/agents/graphs/tools/task-executor.tools.ts(1 hunks)packages/agent/src/agents/graphs/tools/task-manager.tools.ts(1 hunks)packages/agent/src/agents/graphs/utils/graph.utils.ts(1 hunks)packages/agent/src/agents/index.ts(0 hunks)packages/agent/src/agents/operators/agentSelector.ts(0 hunks)packages/agent/src/agents/operators/mcp-agent/mcpAgent.ts(0 hunks)packages/agent/src/agents/operators/mcp-agent/mcpAgentTools.ts(0 hunks)packages/agent/src/agents/operators/operatorRegistry.ts(0 hunks)packages/agent/src/agents/operators/supervisor/supervisorTools.ts(2 hunks)packages/agent/src/agents/operators/supervisor/tools/schemas/common.schemas.ts(1 hunks)packages/agent/src/agents/operators/supervisor/tools/schemas/index.ts(1 hunks)packages/agent/src/agents/operators/supervisor/tools/schemas/searchMcpServer.schema.ts(1 hunks)packages/agent/src/agents/operators/supervisor/tools/searchMcpServerTools.ts(1 hunks)packages/agent/src/agents/studio/studio-graph.ts(1 hunks)packages/agent/src/agents/utils/event.utils.ts(1 hunks)packages/agent/src/index.ts(1 hunks)packages/agent/src/services/mcp/src/__tests__/mcp.spec.ts(1 hunks)packages/agent/src/shared/prompts/agents/config.prompts.ts(0 hunks)packages/agent/src/shared/prompts/agents/constraints-map.ts(0 hunks)packages/agent/src/shared/prompts/agents/index.ts(1 hunks)packages/agent/src/shared/prompts/agents/selector.prompts.ts(0 hunks)packages/agent/src/shared/prompts/agents/snak/core/index.ts(1 hunks)packages/agent/src/shared/prompts/agents/snak/hitl/index.ts(1 hunks)packages/agent/src/shared/prompts/agents/snak/index.ts(1 hunks)packages/agent/src/shared/prompts/agents/supervisor/index.ts(1 hunks)packages/agent/src/shared/prompts/agents/supervisor/specialist/index.ts(1 hunks)packages/agent/src/shared/prompts/core/agentSelectorPrompts.ts(0 hunks)packages/agent/src/shared/prompts/core/configAgentPrompts.ts(0 hunks)packages/agent/src/shared/prompts/core/mcpAgentPrompts.ts(0 hunks)packages/agent/src/shared/prompts/core/prompts.ts(0 hunks)packages/agent/src/shared/prompts/index.ts(1 hunks)packages/agent/src/shared/types/agent.type.ts(1 hunks)packages/agent/src/shared/types/agents.types.ts(0 hunks)packages/agent/src/shared/types/index.ts(1 hunks)packages/agent/src/shared/types/streaming.type.ts(1 hunks)packages/agent/src/shared/types/tools.type.ts(1 hunks)packages/agent/src/shared/types/tools.types.ts(0 hunks)packages/agent/src/tools/tools.ts(1 hunks)packages/core/src/common/agent/interfaces/agent.interface.ts(0 hunks)packages/core/src/common/constant/agents.constants.ts(1 hunks)packages/core/src/common/constant/default-agent.constant.ts(1 hunks)packages/core/src/common/server/dto/agent/config.dto.ts(1 hunks)packages/core/src/common/server/dto/agent/mcp.dto.ts(1 hunks)packages/core/src/common/server/dto/agent/message.dto.ts(1 hunks)packages/core/src/common/server/dto/agents.ts(0 hunks)packages/core/src/common/server/dto/index.ts(1 hunks)packages/core/src/common/server/dto/websocket.ts(0 hunks)packages/core/src/config/database.config.ts(1 hunks)packages/core/src/config/guards/guardsSchema.ts(1 hunks)packages/core/src/index.ts(2 hunks)packages/core/src/services/agent-validation.service.ts(2 hunks)packages/server/common/errors/agent.errors.ts(1 hunks)packages/server/common/errors/application.errors.ts(1 hunks)packages/server/common/errors/base.error.ts(1 hunks)packages/server/common/errors/error.type.ts(0 hunks)packages/server/common/errors/index.ts(1 hunks)packages/server/common/errors/job.errors.ts(1 hunks)packages/server/common/errors/starknet.errors.ts(0 hunks)packages/server/config/configuration.ts(3 hunks)packages/server/config/env.validation.ts(1 hunks)packages/server/mcp.txt(0 hunks)packages/server/src/agents.storage.ts(1 hunks)packages/server/src/controllers/agents.controller.ts(2 hunks)packages/server/src/controllers/gateway.controller.ts(7 hunks)packages/server/src/interfaces/agent-service.interface.ts(0 hunks)packages/server/src/interfaces/agent.interface.ts(0 hunks)packages/server/src/interfaces/sql_interfaces.ts(0 hunks)packages/server/src/interfaces/wallet-service.inferface.ts(0 hunks)packages/server/src/services/agent.service.ts(5 hunks)packages/server/src/utils/agents.utils.ts(2 hunks)packages/workers/package.json(0 hunks)tests/src/file-test/file-upload.ts(1 hunks)tests/src/types.ts(1 hunks)
💤 Files with no reviewable changes (27)
- packages/agent/src/shared/prompts/core/configAgentPrompts.ts
- package.json
- packages/server/common/errors/error.type.ts
- packages/agent/src/shared/prompts/agents/constraints-map.ts
- packages/agent/src/agents/operators/agentSelector.ts
- packages/agent/src/agents/operators/operatorRegistry.ts
- packages/agent/src/shared/prompts/core/mcpAgentPrompts.ts
- packages/server/src/interfaces/agent.interface.ts
- packages/agent/src/shared/prompts/agents/config.prompts.ts
- packages/agent/src/shared/prompts/core/agentSelectorPrompts.ts
- packages/agent/src/agents/index.ts
- packages/agent/src/shared/prompts/agents/selector.prompts.ts
- packages/server/src/interfaces/sql_interfaces.ts
- packages/agent/src/shared/types/tools.types.ts
- packages/agent/src/mocks/@snakagent/core.ts
- packages/server/src/interfaces/agent-service.interface.ts
- packages/core/src/common/agent/interfaces/agent.interface.ts
- packages/server/common/errors/starknet.errors.ts
- packages/server/mcp.txt
- packages/agent/src/agents/operators/mcp-agent/mcpAgent.ts
- packages/core/src/common/server/dto/websocket.ts
- packages/workers/package.json
- packages/core/src/common/server/dto/agents.ts
- packages/agent/src/shared/prompts/core/prompts.ts
- packages/agent/src/shared/types/agents.types.ts
- packages/server/src/interfaces/wallet-service.inferface.ts
- packages/agent/src/agents/operators/mcp-agent/mcpAgentTools.ts
🧰 Additional context used
🧬 Code graph analysis (10)
packages/server/src/services/agent.service.ts (1)
packages/agent/src/shared/types/graph.type.ts (1)
UserRequest(84-87)
packages/server/src/controllers/gateway.controller.ts (2)
packages/core/src/common/server/dto/agent/message.dto.ts (2)
AgentRequestDTO(63-66)MessageFromAgentIdDTO(15-30)packages/core/src/common/server/dto/agent/config.dto.ts (2)
AddAgentRequestDTO(21-24)AgentDeleteRequestDTO(78-83)
packages/core/src/common/server/dto/agent/message.dto.ts (2)
packages/agent/src/agents/index.ts (1)
Message(1-5)packages/server/src/controllers/agents.controller.ts (1)
SupervisorRequestDTO(49-54)
packages/agent/src/agents/core/baseAgent.ts (1)
packages/agent/src/index.ts (1)
BaseAgent(10-10)
packages/agent/src/agents/core/snakAgent.ts (1)
packages/core/src/common/agent/interfaces/agent.interface.ts (1)
Runtime(181-186)
packages/agent/src/agents/operators/supervisor/tools/searchMcpServerTools.ts (1)
packages/agent/src/agents/operators/supervisor/tools/schemas/searchMcpServer.schema.ts (1)
SearchMcpServerSchema(4-24)
packages/core/src/services/agent-validation.service.ts (1)
packages/core/src/services/guards.service.ts (2)
getGuardValue(89-108)getGuardValue(160-162)
packages/server/src/controllers/agents.controller.ts (3)
packages/server/src/utils/controller-helpers.ts (1)
ControllerHelpers(13-155)packages/server/src/utils/error.ts (1)
ServerError(1-42)packages/core/src/common/server/dto/agent/message.dto.ts (1)
MessageRequest(35-50)
packages/agent/src/agents/operators/supervisor/supervisorTools.ts (1)
packages/agent/src/agents/operators/supervisor/tools/searchMcpServerTools.ts (1)
searchMcpServerTool(48-233)
packages/server/src/agents.storage.ts (2)
packages/agent/src/agents/core/snakAgent.ts (1)
SnakAgent(33-434)packages/agent/src/index.ts (1)
SnakAgent(7-7)
🪛 Biome (2.1.2)
tests/src/file-test/file-upload.ts
[error] 26-26: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '~'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 7-7: [UnorderedKey] The POSTGRES_HOST key should go before the POSTGRES_USER key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The POSTGRES_DB key should go before the POSTGRES_HOST key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The POSTGRES_PASSWORD key should go before the POSTGRES_USER key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The POSTGRES_PORT key should go before the POSTGRES_USER key
(UnorderedKey)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [UnorderedKey] The LANGSMITH_ENDPOINT key should go before the LANGSMITH_TRACING key
(UnorderedKey)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [UnorderedKey] The LANGSMITH_API_KEY key should go before the LANGSMITH_ENDPOINT key
(UnorderedKey)
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The LANGSMITH_PROJECT key should go before the LANGSMITH_TRACING key
(UnorderedKey)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 25-25: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 25-25: [UnorderedKey] The REDIS_PASSWORD key should go before the REDIS_PORT key
(UnorderedKey)
[warning] 26-26: [UnorderedKey] The REDIS_DB key should go before the REDIS_HOST key
(UnorderedKey)
[warning] 29-29: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 32-32: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 33-33: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 33-33: [UnorderedKey] The DEFAULT_MODEL_NAME key should go before the DEFAULT_MODEL_PROVIDER key
(UnorderedKey)
[warning] 34-34: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
| "model": { | ||
| "provider_max_length": 100, | ||
| "provider_min_length": 3, | ||
| "model_name_max_length": 100, | ||
| "model_name_min_length": 3, | ||
| "allowed_provider": "gemini", | ||
| "allowed_models": ["gemini-2.5-flash", "gemini-2.5-pro"], | ||
| "max_temperature": 1, | ||
| "max_tokens": 8192 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that schema and validation logic support the new allowlist structure.
The validation strategy has been updated from length-based constraints to explicit allowlists (allowed_provider and allowed_models). This aligns with the PR objective to consolidate to Gemini as the single provider. Ensure that:
- The guards schema (
guardsSchema.ts) properly validates these new fields - The agent validation service correctly enforces the allowlist constraints
- All existing agents using other providers/models are migrated or handled appropriately
Run the following script to verify the guards schema and validation logic:
🏁 Script executed:
#!/bin/bash
# Check guards schema definition
ast-grep --pattern $'allowed_provider'
ast-grep --pattern $'allowed_models'
# Verify validation service enforcement
rg -nP --type=ts -C5 "allowed_provider|allowed_models" packages/core/src/Length of output: 2216
I need to gather more information about the validation enforcement logic and check for any existing agents that might not conform to the new allowlist. Let me run additional verification:
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# See full validation implementation
cat packages/core/src/services/agent-validation.service.ts | sed -n '268,310p'
# Find where schema is applied (look for parse/safeParse of GuardsSchema or AgentGraphModelConfigSchema)
rg -nP --type=ts "parse|safeParse" packages/core/src/config/guards/ -A2 -B2
# Check if there are other model/provider configurations
rg -nP --type=json "model_provider|allowed_provider" config/Length of output: 2717
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Search for other agent configurations with different providers
find config -name "*.json" -type f | xargs grep -l "model_provider\|agents" 2>/dev/null
# Check for any model_provider values other than gemini in config
rg -P "model_provider" config/ -B2 -A2
# Look for agent definitions in graph configs
rg -P "\"agents\"|\"graph\"" config/guards/ -B1 -A1Length of output: 408
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Examine agent configuration files
echo "=== default.agent.json ==="
cat config/agents/default.agent.json | head -50
echo -e "\n=== supervisor.agent.json ==="
cat config/agents/supervisor.agent.json | head -50
# Look for model_provider in agent configs specifically
rg -P "model_provider" config/agents/ -B2 -A2Length of output: 4473
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Find where validateModelConfig is called
rg -nP "validateModelConfig" packages/core/src/ -B3 -A3
# Check if agents are loaded and validated during startup
rg -nP "loadGuardsConfig|validateAgent|validateModelConfig" packages/core/src/ | head -30
# See if there's any migration logic or fallback for existing agents
rg -nP "migrate|fallback|default.*model|openai" config/agents/ -B2 -A2Length of output: 5219
Migrate all existing agents to use Gemini models before deployment.
The guards schema and validation enforcement are correctly implemented, but all 7 existing agent configurations still use OpenAI (provider="openai", model_name="gpt-4o"). When validateAgent() is called, validation will fail with "Invalid model provider. Only 'gemini' is supported."
Update the following agent files to use "provider": "gemini" and a supported model from allowed_models (e.g., "gemini-2.5-flash" or "gemini-2.5-pro"):
- config/agents/default.agent.json
- config/agents/supervisor.agent.json
- config/agents/starknet-rpc.agent.json
- config/agents/memory.agent.json
- config/agents/example.agent.json
- config/agents/ethereum-rpc.agent.json
- config/agents/cairo.agent.json
🤖 Prompt for AI Agents
In config/guards/default.guards.json around lines 95-100 the guard restricts
models to provider "gemini" and allowed_models
["gemini-2.5-flash","gemini-2.5-pro"], but seven agent configs still reference
OpenAI, causing validateAgent() to fail; open each of these
files—config/agents/default.agent.json, supervisor.agent.json,
starknet-rpc.agent.json, memory.agent.json, example.agent.json,
ethereum-rpc.agent.json, cairo.agent.json—and change the model provider field to
"gemini" and the model_name (or model) to a supported value such as
"gemini-2.5-flash" (or "gemini-2.5-pro"), save, and re-run validation to ensure
no schema errors remain.
| model_name: z | ||
| .string() | ||
| .min(modelGuardsValues.model_name_min_length) | ||
| .max(modelGuardsValues.model_name_max_length) | ||
| .refine((val) => modelGuardsValues.allowed_models.includes(val), { | ||
| message: `Model must be one of: ${modelGuardsValues.allowed_models.join(', ')}`, | ||
| }) | ||
| .optional() | ||
| .describe('Model name'), |
There was a problem hiding this comment.
Critical: Optional field rejects undefined values.
The refine check validates against allowed_models without accounting for undefined. When model_name is not provided (which should be valid since it's optional), the validation fails because includes(undefined) returns false.
Apply this diff to allow undefined values:
model_name: z
.string()
.refine(
- (val) => modelGuardsValues.allowed_models.includes(val),
+ (val) => !val || modelGuardsValues.allowed_models.includes(val),
{
message: `Model must be one of: ${modelGuardsValues.allowed_models.join(', ')}`,
}
)
.optional()
.describe('Model name'),Alternatively, move optional() before refine() and add a type guard:
model_name: z
.string()
+ .optional()
.refine(
- (val) => modelGuardsValues.allowed_models.includes(val),
+ (val) => val === undefined || modelGuardsValues.allowed_models.includes(val),
{
message: `Model must be one of: ${modelGuardsValues.allowed_models.join(', ')}`,
}
)
- .optional()
.describe('Model name'),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| model_name: z | |
| .string() | |
| .min(modelGuardsValues.model_name_min_length) | |
| .max(modelGuardsValues.model_name_max_length) | |
| .refine((val) => modelGuardsValues.allowed_models.includes(val), { | |
| message: `Model must be one of: ${modelGuardsValues.allowed_models.join(', ')}`, | |
| }) | |
| .optional() | |
| .describe('Model name'), | |
| model_name: z | |
| .string() | |
| .refine( | |
| (val) => val === undefined || modelGuardsValues.allowed_models.includes(val), | |
| { | |
| message: `Model must be one of: ${modelGuardsValues.allowed_models.join(', ')}`, | |
| } | |
| ) | |
| .optional() | |
| .describe('Model name'), |
| model_name: z | |
| .string() | |
| .min(modelGuardsValues.model_name_min_length) | |
| .max(modelGuardsValues.model_name_max_length) | |
| .refine((val) => modelGuardsValues.allowed_models.includes(val), { | |
| message: `Model must be one of: ${modelGuardsValues.allowed_models.join(', ')}`, | |
| }) | |
| .optional() | |
| .describe('Model name'), | |
| model_name: z | |
| .string() | |
| .optional() | |
| .refine( | |
| (val) => val === undefined || modelGuardsValues.allowed_models.includes(val), | |
| { | |
| message: `Model must be one of: ${modelGuardsValues.allowed_models.join(', ')}`, | |
| } | |
| ) | |
| .describe('Model name'), |
🤖 Prompt for AI Agents
In
packages/agent/src/agents/operators/supervisor/tools/schemas/common.schemas.ts
around lines 50 to 56, the current refine check runs before the field is marked
optional so undefined fails validation; change the schema so undefined is
allowed either by moving .optional() before .refine() and making the refine
predicate a type guard that returns true for undefined, or keep .optional() last
but change the refine predicate to (val) => val === undefined ||
modelGuardsValues.allowed_models.includes(val) so undefined passes validation
while still validating provided strings.
| installation: { | ||
| isRemote: server.isDeployed && httpConnection, | ||
| requiresApiKey: server.isDeployed, | ||
| hasLocalOption: !!stdioConnection, | ||
| configurationRequired: !!( | ||
| httpConnection?.configSchema?.required?.length || | ||
| stdioConnection?.configSchema?.required?.length | ||
| ), | ||
| }, |
There was a problem hiding this comment.
Boolean field bug: isRemote becomes an object; coerce to boolean
server.isDeployed && httpConnection returns the connection object, not a boolean.
- connections: detail.connections.map((conn) => ({
+ connections: detail.connections.map((conn) => ({
type: conn.type,
url: conn.url,
hasConfig: !!(
conn.configSchema?.properties &&
Object.keys(conn.configSchema.properties).length > 0
),
requiredFields: conn.configSchema?.required || [],
configFields: conn.configSchema?.properties
? Object.keys(conn.configSchema.properties)
: [],
})),
installation: {
- isRemote: server.isDeployed && httpConnection,
+ isRemote: !!(server.isDeployed && httpConnection),
requiresApiKey: server.isDeployed,
hasLocalOption: !!stdioConnection,
configurationRequired: !!(
httpConnection?.configSchema?.required?.length ||
stdioConnection?.configSchema?.required?.length
),
},Also applies to: 167-178
| // /** File deprecated **/ | ||
| // import { Graph } from '@agents/graphs/core-graph/agent.graph.js'; | ||
| // import { BaseChatModel } from '@langchain/core/language_models/chat_models'; | ||
| // import { AgentConfig } from '@snakagent/core'; | ||
| // import { Postgres } from '@snakagent/database'; | ||
| // import { ChatOpenAI } from '@langchain/openai'; | ||
| // import { ChatAnthropic } from '@langchain/anthropic'; | ||
| // import { ChatGoogleGenerativeAI } from '@langchain/google-genai'; | ||
| // import { SnakAgent } from '@agents/core/snakAgent.js'; | ||
| // import { | ||
| // TASK_EXECUTOR_SYSTEM_PROMPT, | ||
| // TASK_MANAGER_SYSTEM_PROMPT, | ||
| // TASK_MEMORY_MANAGER_SYSTEM_PROMPT, | ||
| // TASK_VERIFIER_SYSTEM_PROMPT, | ||
| // } from '@prompts/index.js'; | ||
| // async function getAgentConfigFromId(agentId: string) { | ||
| // await Postgres.connect({ | ||
| // database: process.env.POSTGRES_DB as string, | ||
| // host: process.env.POSTGRES_HOST as string, | ||
| // user: process.env.POSTGRES_USER as string, | ||
| // password: process.env.POSTGRES_PASSWORD as string, | ||
| // port: parseInt(process.env.POSTGRES_PORT as string), | ||
| // }); | ||
| // const query = new Postgres.Query( | ||
| // ` | ||
| // SELECT | ||
| // id, | ||
| // user_id, | ||
| // row_to_json(profile) as profile, | ||
| // mcp_servers as "mcp_servers", | ||
| // prompts_id, | ||
| // row_to_json(graph) as graph, | ||
| // row_to_json(memory) as memory, | ||
| // row_to_json(rag) as rag, | ||
| // FROM agents | ||
| // WHERE id = $1 | ||
| // `, | ||
| // [agentId] | ||
| // ); | ||
| // const result = await Postgres.query<AgentConfig.OutputWithId>(query); | ||
| // if (result.length === 0) { | ||
| // throw new Error(`Agent with ID ${agentId} not found`); | ||
| // } | ||
| // return result[0]; | ||
| // } | ||
| // /** | ||
| // * Create an autonomous agent graph | ||
| // * @param agentId - The unique identifier of the agent | ||
| // * @returns Promise<any> - The initialized graph | ||
| // */ | ||
| // export async function createAutonomousAgent(agentId: string): Promise<any> { | ||
| // const agent_config = await getAgentConfigFromId(agentId); | ||
| // if (!agent_config) { | ||
| // throw new Error(`Agent with ID ${agentId} not found`); | ||
| // } | ||
| // let model = process.env.DEFAULT_MODEL_PROVIDER; | ||
| // if (!model) { | ||
| // throw new Error('Model configuration is not defined'); | ||
| // } | ||
| // let modelInstance: BaseChatModel | null = null; | ||
| // const commonConfig = { | ||
| // modelName: process.env.DEFAULT_MODEL_NAME as string, | ||
| // verbose: false, | ||
| // temperature: parseFloat(process.env.DEFAULT_TEMPERATURE ?? '0.7'), | ||
| // }; | ||
| // switch (model.toLowerCase()) { | ||
| // case 'openai': | ||
| // modelInstance = new ChatOpenAI({ | ||
| // ...commonConfig, | ||
| // openAIApiKey: process.env.OPENAI_API_KEY, | ||
| // }); | ||
| // break; | ||
| // case 'anthropic': | ||
| // modelInstance = new ChatAnthropic({ | ||
| // ...commonConfig, | ||
| // anthropicApiKey: process.env.ANTHROPIC_API_KEY, | ||
| // }); | ||
| // break; | ||
| // case 'gemini': | ||
| // modelInstance = new ChatGoogleGenerativeAI({ | ||
| // model: commonConfig.modelName, // Updated to valid Gemini model name | ||
| // verbose: commonConfig.verbose, | ||
| // temperature: commonConfig.temperature, | ||
| // apiKey: process.env.GEMINI_API_KEY, | ||
| // }); | ||
| // break; | ||
| // // Add case for 'deepseek' if a Langchain integration exists or becomes available | ||
| // default: | ||
| // throw new Error('No valid model provided'); | ||
| // } | ||
|
|
||
| const agent: AgentConfig.Runtime = { | ||
| ...agent_config, | ||
| prompts: { | ||
| task_executor_prompt: TASK_EXECUTOR_SYSTEM_PROMPT, | ||
| task_manager_prompt: TASK_MANAGER_SYSTEM_PROMPT, | ||
| task_memory_manager_prompt: TASK_MEMORY_MANAGER_SYSTEM_PROMPT, | ||
| task_verifier_prompt: TASK_VERIFIER_SYSTEM_PROMPT, | ||
| }, | ||
| graph: { | ||
| ...agent_config.graph, | ||
| model: modelInstance, | ||
| }, | ||
| }; | ||
| const starknetConfig: StarknetConfig = { | ||
| provider: this.config.starknet.provider, | ||
| accountPrivateKey: this.config.starknet.privateKey, | ||
| accountPublicKey: this.config.starknet.publicKey, | ||
| }; | ||
| const snakAgent: SnakAgent = new SnakAgent(starknetConfig, agent, { | ||
| database: process.env.POSTGRES_DB as string, | ||
| host: process.env.POSTGRES_HOST as string, | ||
| user: process.env.POSTGRES_USER as string, | ||
| password: process.env.POSTGRES_PASSWORD as string, | ||
| port: parseInt(process.env.POSTGRES_PORT as string), | ||
| }); | ||
| const autonomousAgent = new Graph(snakAgent); | ||
| const app = await autonomousAgent.initialize(); | ||
| return app; | ||
| } | ||
| // const agent: AgentConfig.Runtime = { | ||
| // ...agent_config, | ||
| // prompts: { | ||
| // task_executor_prompt: TASK_EXECUTOR_SYSTEM_PROMPT, | ||
| // task_manager_prompt: TASK_MANAGER_SYSTEM_PROMPT, | ||
| // task_memory_manager_prompt: TASK_MEMORY_MANAGER_SYSTEM_PROMPT, | ||
| // task_verifier_prompt: TASK_VERIFIER_SYSTEM_PROMPT, | ||
| // }, | ||
| // graph: { | ||
| // ...agent_config.graph, | ||
| // model: modelInstance, | ||
| // }, | ||
| // }; | ||
| // const snakAgent: SnakAgent = new SnakAgent(starknetConfig, agent, { | ||
| // database: process.env.POSTGRES_DB as string, | ||
| // host: process.env.POSTGRES_HOST as string, | ||
| // user: process.env.POSTGRES_USER as string, | ||
| // password: process.env.POSTGRES_PASSWORD as string, | ||
| // port: parseInt(process.env.POSTGRES_PORT as string), | ||
| // }); | ||
| // const autonomousAgent = new Graph(snakAgent); | ||
| // const app = await autonomousAgent.initialize(); | ||
| // return app; | ||
| // } | ||
|
|
||
| // Example usage with specific IDs (for backward compatibility) | ||
| const AUTONOMOUS_ID = '223d72b7-7b61-43af-bbf6-278e69994b3f'; | ||
| // // Example usage with specific IDs (for backward compatibility) | ||
| // const AUTONOMOUS_ID = '223d72b7-7b61-43af-bbf6-278e69994b3f'; | ||
|
|
||
| export const studio_graph_autonomous = () => | ||
| createAutonomousAgent(AUTONOMOUS_ID); | ||
| // export const studio_graph_autonomous = () => | ||
| // createAutonomousAgent(AUTONOMOUS_ID); |
There was a problem hiding this comment.
🧩 Analysis chain
Delete this deprecated file entirely instead of leaving commented code.
The entire file is commented out, which adds maintenance burden and confusion. Since the PR objectives include cleaning dead code and removing Starknet-related functionality, this file should be deleted completely. Version control preserves the history if needed in the future.
Additionally, line 105 references an undefined starknetConfig variable that was never imported or declared, suggesting this code was already non-functional.
Verify that no other files import from this deprecated file:
🏁 Script executed:
#!/bin/bash
# Search for imports of this file or its exports
rg -n --type=ts --type=js -C3 "from ['\"].*studio-graph" -g '!**/studio-graph.ts'
rg -n --type=ts --type=js -C3 "studio_graph_autonomous|createAutonomousAgent" -g '!**/studio-graph.ts'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Verify file exists and check directory structure
echo "=== Checking file existence ==="
ls -la packages/agent/src/agents/studio/studio-graph.ts 2>/dev/null && echo "File exists" || echo "File NOT found"
echo -e "\n=== Checking for barrel/index files in studio directory ==="
find packages/agent/src/agents/studio -name "index.*" -o -name "*.barrel.*" 2>/dev/null | head -20
echo -e "\n=== Checking parent directory for re-exports ==="
find packages/agent/src/agents -maxdepth 1 -name "index.*" 2>/dev/null | xargs grep -l "studio-graph" 2>/dev/null || echo "No re-exports found"
echo -e "\n=== Broader search for studio-graph or functions ==="
rg -i "studio.graph|studio_graph_autonomous" --type=ts --type=js -g '!**/studio-graph.ts' 2>/dev/null | head -20 || echo "No matches found"
echo -e "\n=== Checking for dynamic imports or requires ==="
rg "require.*studio|import.*studio" --type=ts --type=js -g '!**/studio-graph.ts' 2>/dev/null | head -20 || echo "No dynamic imports found"Length of output: 376
Delete the entire deprecated file packages/agent/src/agents/studio/studio-graph.ts.
Verification confirms this file contains no external references from anywhere in the codebase. The file is completely dead code and safe to remove entirely rather than leaving it commented out. This aligns with the PR objectives of removing Starknet-related code and cleaning up dead code.
🤖 Prompt for AI Agents
packages/agent/src/agents/studio/studio-graph.ts lines 1-121: this file is
deprecated and entirely commented out; delete the file from the repo. Remove any
direct import/exports or barrel exports referencing it (if present) and update
package.json/tsconfig/linters if the file path was explicitly listed; then run
the repo typecheck/build and tests to confirm no remaining references.
| * Re-export all HITL prompts | ||
| */ | ||
|
|
||
| export * from './hitl-contraint.prompt.js'; |
There was a problem hiding this comment.
Fix typo in the source filename.
The file being imported is named hitl-contraint.prompt.js, which contains a typo. It should be hitl-constraint.prompt.js (with an "s"). Since this PR focuses on cleaning the codebase and improving naming, this typo should be corrected.
Rename the source file and update the import:
-export * from './hitl-contraint.prompt.js';
+export * from './hitl-constraint.prompt.js';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/agent/src/shared/prompts/agents/snak/hitl/index.ts around line 5,
the exported source filename contains a typo: it currently exports
'./hitl-contraint.prompt.js' but the correct filename is
'./hitl-constraint.prompt.js'; rename the actual file on disk to
hitl-constraint.prompt.js (fixing the "contraint"->"constraint") and update this
export statement to reference the corrected filename, then run project-wide
searches for any other imports referencing the misspelled filename and update
them accordingly.
| @IsOptional() | ||
| @IsInt() | ||
| @Min(0) | ||
| @Max(1) | ||
| hitl_threshold?: number; |
There was a problem hiding this comment.
Fix validation decorator mismatch.
The hitl_threshold field is decorated with @IsInt() but has a range of 0-1, which suggests it should be a decimal value (float/number). Integer validation will reject any decimal values like 0.5, 0.7, etc.
Apply this diff to fix the validation:
@IsOptional()
- @IsInt()
@Min(0)
@Max(1)
hitl_threshold?: number;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @IsOptional() | |
| @IsInt() | |
| @Min(0) | |
| @Max(1) | |
| hitl_threshold?: number; | |
| @IsOptional() | |
| @Min(0) | |
| @Max(1) | |
| hitl_threshold?: number; |
🤖 Prompt for AI Agents
In packages/core/src/common/server/dto/agent/message.dto.ts around lines 45 to
49, the hitl_threshold field is currently validated with @IsInt() but the
allowed range 0–1 implies a decimal; replace the integer validator with a
numeric validator (e.g., use @IsNumber() with appropriate options) and keep the
@IsOptional(), @Min(0) and @Max(1) decorators so decimal values like 0.5 are
accepted while still enforcing the 0–1 range.
| // Default model configuration | ||
| DEFAULT_MODEL_PROVIDER: z.string(), | ||
| DEFAULT_MODEL_NAME: z.string(), | ||
| DEFAULT_TEMPERATURE: z.coerce.number().min(0).max(1), |
There was a problem hiding this comment.
🧩 Analysis chain
Verify DEFAULT_MAX_TOKENS environment variable.
The default model configuration is defined here, but packages/server/src/agents.storage.ts (line 393) also references DEFAULT_MAX_TOKENS environment variable, which is not included in this schema. Ensure this variable is either added to the schema or removed from usage to prevent runtime errors.
Run the following script to verify the usage of DEFAULT_MAX_TOKENS:
🏁 Script executed:
#!/bin/bash
# Search for DEFAULT_MAX_TOKENS usage in the codebase
rg -n -C3 '\bDEFAULT_MAX_TOKENS\b'Length of output: 697
🏁 Script executed:
#!/bin/bash
# Check if DEFAULT_MAX_TOKENS is defined in env.validation.ts
rg -n '\bDEFAULT_MAX_TOKENS\b' packages/server/config/env.validation.ts
# Also list all DEFAULT_* environment variables in the schema
echo "---"
echo "All DEFAULT_* variables in env schema:"
rg -n 'DEFAULT_[A-Z_]+:' packages/server/config/env.validation.tsLength of output: 179
Add DEFAULT_MAX_TOKENS to environment schema.
DEFAULT_MAX_TOKENS is actively used in packages/server/src/agents.storage.ts (line 393) but is missing from the validation schema. Add it to maintain type safety and ensure all environment variables are properly validated:
DEFAULT_MAX_TOKENS: z.coerce.number().int().min(1),
🤖 Prompt for AI Agents
In packages/server/config/env.validation.ts around lines 20 to 23, the schema is
missing DEFAULT_MAX_TOKENS which is used elsewhere; add a new entry
DEFAULT_MAX_TOKENS: z.coerce.number().int().min(1) to the exported Zod schema so
the env var is validated as an integer >=1 and included in the typed
environment.
| const modelInstance = new ChatGoogleGenerativeAI({ | ||
| model: model.model_name, | ||
| verbose: false, | ||
| temperature: model.temperature, | ||
| }; | ||
| switch (model.provider.toLowerCase()) { | ||
| case 'openai': | ||
| modelInstance = new ChatOpenAI({ | ||
| ...commonConfig, | ||
| openAIApiKey: process.env.OPENAI_API_KEY, | ||
| }); | ||
| break; | ||
| case 'anthropic': | ||
| modelInstance = new ChatAnthropic({ | ||
| ...commonConfig, | ||
| anthropicApiKey: process.env.ANTHROPIC_API_KEY, | ||
| }); | ||
| break; | ||
| case 'gemini': | ||
| modelInstance = new ChatGoogleGenerativeAI({ | ||
| model: model.model_name, // Updated to valid Gemini model name | ||
| verbose: false, | ||
| temperature: model.temperature, | ||
| apiKey: process.env.GEMINI_API_KEY, | ||
| }); | ||
| break; | ||
| // Add case for 'deepseek' if a Langchain integration exists or becomes available | ||
| default: | ||
| throw new Error('No valid model provided'); | ||
| } | ||
| apiKey: process.env.GEMINI_API_KEY, | ||
| }); |
There was a problem hiding this comment.
Add explicit validation for GEMINI_API_KEY environment variable.
The PR objectives mention "improved environment security: checks and more explicit errors when required env fields are missing," but GEMINI_API_KEY is used without validation. If undefined or empty, this will cause unclear runtime failures.
Apply this diff to add explicit validation:
+ // Validate API key
+ if (!process.env.GEMINI_API_KEY) {
+ throw new Error(
+ 'GEMINI_API_KEY environment variable is required but not defined.'
+ );
+ }
+
const modelInstance = new ChatGoogleGenerativeAI({
model: model.model_name,
verbose: false,
temperature: model.temperature,
apiKey: process.env.GEMINI_API_KEY,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const modelInstance = new ChatGoogleGenerativeAI({ | |
| model: model.model_name, | |
| verbose: false, | |
| temperature: model.temperature, | |
| }; | |
| switch (model.provider.toLowerCase()) { | |
| case 'openai': | |
| modelInstance = new ChatOpenAI({ | |
| ...commonConfig, | |
| openAIApiKey: process.env.OPENAI_API_KEY, | |
| }); | |
| break; | |
| case 'anthropic': | |
| modelInstance = new ChatAnthropic({ | |
| ...commonConfig, | |
| anthropicApiKey: process.env.ANTHROPIC_API_KEY, | |
| }); | |
| break; | |
| case 'gemini': | |
| modelInstance = new ChatGoogleGenerativeAI({ | |
| model: model.model_name, // Updated to valid Gemini model name | |
| verbose: false, | |
| temperature: model.temperature, | |
| apiKey: process.env.GEMINI_API_KEY, | |
| }); | |
| break; | |
| // Add case for 'deepseek' if a Langchain integration exists or becomes available | |
| default: | |
| throw new Error('No valid model provided'); | |
| } | |
| apiKey: process.env.GEMINI_API_KEY, | |
| }); | |
| // Validate API key | |
| if (!process.env.GEMINI_API_KEY) { | |
| throw new Error( | |
| 'GEMINI_API_KEY environment variable is required but not defined.' | |
| ); | |
| } | |
| const modelInstance = new ChatGoogleGenerativeAI({ | |
| model: model.model_name, | |
| verbose: false, | |
| temperature: model.temperature, | |
| apiKey: process.env.GEMINI_API_KEY, | |
| }); |
🤖 Prompt for AI Agents
In packages/server/src/utils/agents.utils.ts around lines 33 to 38, the
GEMINI_API_KEY is used directly when constructing ChatGoogleGenerativeAI; add an
explicit check that process.env.GEMINI_API_KEY exists and is non-empty before
creating modelInstance, and if it is missing throw or return a clear,
descriptive error (e.g., throw new Error('GEMINI_API_KEY environment variable is
required for ChatGoogleGenerativeAI')); then pass the validated key (not the raw
env access) to the ChatGoogleGenerativeAI constructor to ensure failures surface
immediately with a helpful message.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/server/src/utils/agents.utils.ts (1)
34-39: Add explicit validation for GEMINI_API_KEY environment variable.The PR objectives mention "improved environment security: checks and more explicit errors when required env fields are missing," but
GEMINI_API_KEYis used without validation. If undefined or empty, this will cause unclear runtime failures.Apply this diff to add explicit validation:
+ // Validate API key + if (!process.env.GEMINI_API_KEY) { + throw new Error( + 'GEMINI_API_KEY environment variable is required but not defined.' + ); + } + const modelInstance = new ChatGoogleGenerativeAI({ model: model.model_name, verbose: false, temperature: model.temperature, apiKey: process.env.GEMINI_API_KEY, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/agent/src/agents/graphs/utils/graph.utils.ts(1 hunks)packages/agent/src/agents/operators/supervisor/tools/schemas/common.schemas.ts(1 hunks)packages/agent/src/shared/prompts/agents/snak/hitl/index.ts(1 hunks)packages/core/src/common/server/dto/agent/message.dto.ts(1 hunks)packages/server/config/configuration.ts(3 hunks)packages/server/src/controllers/agents.controller.ts(2 hunks)packages/server/src/utils/agents.utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/agent/src/agents/graphs/utils/graph.utils.ts
- packages/agent/src/shared/prompts/agents/snak/hitl/index.ts
- packages/core/src/common/server/dto/agent/message.dto.ts
- packages/server/config/configuration.ts
- packages/server/src/controllers/agents.controller.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: validate-build
🔇 Additional comments (5)
packages/server/src/utils/agents.utils.ts (4)
20-25: LGTM! Clear provider restriction.The provider validation correctly restricts to Gemini only with a clear error message, aligning with the PR objectives to consolidate model/provider support.
27-32: LGTM! Explicit model validation.The model name validation is well-implemented and provides a clear, explicit error message listing all supported models.
42-47: LGTM! Appropriate error handling.The error handling correctly logs failures with context and returns null, matching the function's return type signature.
5-5: Model names are valid. The model namesgemini-2.5-proandgemini-2.5-flashare confirmed valid per Google's official Gemini API documentation.packages/agent/src/agents/operators/supervisor/tools/schemas/common.schemas.ts (1)
50-55: No issues found—model_name field is appropriately required.All
ModelConfiginstantiations in the codebase providemodel_name(either directly via environment variables, defaults, or normalization functions). Making this field required prevents the undefined validation issues flagged in the past review. This is the correct fix.
| provider: z | ||
| .string() | ||
| .min(modelGuardsValues.provider_min_length) | ||
| .max(modelGuardsValues.provider_max_length) | ||
| .refine( | ||
| (val) => | ||
| val.toLowerCase() === modelGuardsValues.allowed_provider.toLowerCase(), | ||
| { | ||
| message: `Provider must be '${modelGuardsValues.allowed_provider}'`, | ||
| } | ||
| ) | ||
| .optional() | ||
| .describe('Model provider'), |
There was a problem hiding this comment.
Critical: Optional field will throw TypeError on undefined values.
The provider field has the same issue previously flagged for model_name. When provider is not provided (valid since it's optional), the refine callback receives undefined and val.toLowerCase() throws a TypeError.
Apply this diff to handle undefined values:
provider: z
.string()
+ .optional()
.refine(
- (val) =>
- val.toLowerCase() === modelGuardsValues.allowed_provider.toLowerCase(),
+ (val) =>
+ val === undefined || val.toLowerCase() === modelGuardsValues.allowed_provider.toLowerCase(),
{
message: `Provider must be '${modelGuardsValues.allowed_provider}'`,
}
)
- .optional()
.describe('Model provider'),Alternatively, add a guard in the refine predicate while keeping .optional() last:
provider: z
.string()
.refine(
- (val) =>
- val.toLowerCase() === modelGuardsValues.allowed_provider.toLowerCase(),
+ (val) =>
+ !val || val.toLowerCase() === modelGuardsValues.allowed_provider.toLowerCase(),
{
message: `Provider must be '${modelGuardsValues.allowed_provider}'`,
}
)
.optional()
.describe('Model provider'),🤖 Prompt for AI Agents
In
packages/agent/src/agents/operators/supervisor/tools/schemas/common.schemas.ts
around lines 39 to 49, the refine predicate calls val.toLowerCase() which throws
when provider is undefined because the field is optional; update the predicate
to first guard for undefined/null and only call toLowerCase when val is a string
(e.g., if val == null return true or use typeof val === 'string' before
comparing), or move .optional() before the .refine so refine never runs for
undefined — implement one of these fixes so no TypeError occurs when provider is
omitted.
| # --- General Agent Configuration (mandatory) --- | ||
| SERVER_API_KEY="YOUR_SERVER_API_KEY" # A secret key for your agent server API | ||
| SERVER_PORT="3001" | ||
| SERVER_API_KEY="YOUR_SERVER_API_KEY" |
There was a problem hiding this comment.
SERVER_API_KEY is deprecated
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/server/src/utils/agents.utils.ts (1)
39-44: GEMINI_API_KEY still lacks explicit validation.As noted in the previous review, the
GEMINI_API_KEYenvironment variable is used without validation. Given the PR objectives emphasize "improved environment security: checks and more explicit errors when required env fields are missing," this should be addressed.
🧹 Nitpick comments (3)
config/agents/default.agent.json (1)
3-26: Minor: Agent description still references Starknet.The PR objective states "Remove everything related to StarkNet," but the default agent description and contexts extensively mention Starknet (lines 6, 10, 20). Consider whether this documentation should be updated or if Starknet references are intentional given the project's foundation.
packages/server/src/utils/agents.utils.ts (2)
10-10: Consider usingas constfor better type safety.The
SUPPORTED_GEMINI_MODELSarray could be defined withas constto enable TypeScript to infer literal types, which provides better type checking and autocomplete.Apply this diff:
-const SUPPORTED_GEMINI_MODELS = ['gemini-2.5-flash', 'gemini-2.5-pro']; +const SUPPORTED_GEMINI_MODELS = ['gemini-2.5-flash', 'gemini-2.5-pro'] as const;
72-74: Remove unnecessary error handling.The try-catch block that simply re-throws the error adds no value. Either handle the error meaningfully or remove the try-catch entirely.
Apply this diff:
export function initializeAgentConfigIfMissingParams( agentConfig: AgentConfig.InputWithPartialConfig ): AgentConfig.Input { - try { - if (!agentConfig.mcp_servers) { - agentConfig.mcp_servers = AgentConfigDefaults.mcp_servers; - } - if (!agentConfig.memory) { - agentConfig.memory = AgentConfigDefaults.memory; - } - if (!agentConfig.rag) { - agentConfig.rag = AgentConfigDefaults.rag; - } - if (!agentConfig.graph) { - agentConfig.graph = AgentConfigDefaults.graph; - } - return agentConfig as AgentConfig.Input; - } catch (error) { - throw error; - } + if (!agentConfig.mcp_servers) { + agentConfig.mcp_servers = AgentConfigDefaults.mcp_servers; + } + if (!agentConfig.memory) { + agentConfig.memory = AgentConfigDefaults.memory; + } + if (!agentConfig.rag) { + agentConfig.rag = AgentConfigDefaults.rag; + } + if (!agentConfig.graph) { + agentConfig.graph = AgentConfigDefaults.graph; + } + return agentConfig as AgentConfig.Input; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
config/agents/cairo.agent.json(1 hunks)config/agents/default.agent.json(1 hunks)config/agents/ethereum-rpc.agent.json(1 hunks)config/agents/example.agent.json(1 hunks)config/agents/memory.agent.json(1 hunks)config/agents/starknet-rpc.agent.json(1 hunks)config/agents/supervisor.agent.json(1 hunks)config/models/default.models.json(1 hunks)packages/core/src/common/agent/interfaces/agent.interface.ts(1 hunks)packages/core/src/common/constant/agents.constants.ts(2 hunks)packages/core/src/common/server/dto/agent/config.dto.ts(2 hunks)packages/server/src/controllers/agents.controller.ts(4 hunks)packages/server/src/utils/agents.utils.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/server/src/controllers/agents.controller.ts
- packages/core/src/common/server/dto/agent/config.dto.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/server/src/utils/agents.utils.ts (1)
packages/core/src/common/agent/interfaces/agent.interface.ts (2)
InputWithPartialConfig(154-159)Input(145-148)
packages/core/src/common/agent/interfaces/agent.interface.ts (1)
packages/agent/src/agents/operators/supervisor/tools/schemas/common.schemas.ts (2)
MemoryConfig(286-286)RAGConfig(287-287)
🪛 ESLint
packages/core/src/common/constant/agents.constants.ts
[error] 130-173: ES2015 module syntax is preferred over namespaces.
(@typescript-eslint/no-namespace)
🔇 Additional comments (12)
config/agents/supervisor.agent.json (1)
27-28: Model provider migration is consistent.The Gemini 2.5 Flash provider/model update aligns with the repository-wide migration pattern.
config/agents/starknet-rpc.agent.json (1)
1-34: Potential misalignment with PR objectives: Starknet agent still present.The PR objectives state "Remove everything related to StarkNet," but this agent config file remains. Clarify whether this file should be:
- Deleted entirely (align with stated cleanup goals)
- Retained as-is (with the Gemini model update applied here)
If retention is intentional, the model migration is correct. If deletion is planned, this should be removed.
config/agents/ethereum-rpc.agent.json (1)
48-49: Model provider migration is consistent and appropriate.The Gemini 2.5 Flash provider/model update aligns with the repository-wide migration pattern.
config/agents/cairo.agent.json (1)
1-39: Potential misalignment with PR objectives: Cairo agent still present.The PR objectives state "Remove everything related to StarkNet," but this Cairo agent (which is Starknet-specific tooling) remains with only the model updated. Clarify whether this file should be:
- Deleted entirely (align with stated cleanup goals)
- Retained as-is (with the Gemini model update applied)
If retention is intentional, the model migration is correct. If deletion is planned, this should be removed.
config/agents/default.agent.json (1)
43-46: Model provider migration is correct.The Gemini 2.5 Flash provider/model update is properly applied.
config/agents/memory.agent.json (1)
25-26: Model provider migration is consistent and correct.The Gemini 2.5 Flash provider/model update aligns with the repository-wide migration pattern.
config/models/default.models.json (1)
2-22: Model presets uniformly migrated to Gemini; token limits need verification.All three presets (fast, smart, cheap) are correctly updated to google-genai/gemini-2.5-flash. However,
max_input_tokensandmax_output_tokensare set to 0 across all presets. Confirm whether this represents:
- Unlimited tokens (expected behavior)
- Placeholder values (require proper Gemini token limits)
- Config override (values set elsewhere)
If these should have actual limits, Gemini 2.5 Flash models have specific token windows that should be enforced.
packages/server/src/utils/agents.utils.ts (3)
3-8: LGTM! Clean import consolidation.The imports are now properly consolidated from
@snakagent/core, which aligns with the PR objective of cleaning the codebase structure.
25-37: LGTM! Clear and defensive validation.The provider and model validation logic is well-implemented with descriptive error messages that clearly communicate the constraints and supported options.
68-71: I need to verify theInputinterface definition to check ifgraphis actually required. Let me search the codebase for that.Based on the codebase output and web search results, I can now analyze the review comment accurately. The search results confirmed Omit takes a union type of the keys to be omitted as the second argument and returns the derived type that excludes those properties. Since
InputWithPartialConfigextendsOmit<Input, 'memory' | 'mcp_servers' | 'rag'>, thegraphfield is NOT omitted and therefore inherits its type directly fromInput.However, the review comment makes a critical assumption that needs verification. The code shows a check for a missing
graphfield at lines 68-70, but without seeing the actualInputinterface definition to confirm whethergraphis truly required, the review raises a valid concern about potential type misalignment. The pattern of checking for a missing required field and providing a default suggests there may be an intentional gap between the runtime expectation (graph might be optional) and the type definition (graph appears to be required).The review comment cannot be definitively resolved without accessing the
Inputinterface definition. The codebase search didn't return the fullInputinterface, only theInputWithPartialConfigextension. Additionally, the original review comment doesn't provide evidence that TypeScript itself is flagging this as a type error, and it's possible the type cast is actually valid depending on howInputis defined and what strictness settings are enabled.packages/core/src/common/agent/interfaces/agent.interface.ts (1)
150-159: LGTM! Clean type design for partial configuration updates.The new
InputWithPartialConfiginterface properly handles the use case where memory, rag, and mcp_servers configurations should be optional or partially specified during updates. The use ofOmitandPartialis appropriate here.packages/core/src/common/constant/agents.constants.ts (1)
130-173: No changes required—namespace pattern aligns with established codebase conventions.The
export namespacepattern used here is consistent with the project's established patterns. The codebase usesexport namespaceextensively across multiple packages (AgentConfig, MemoryStateManager, Postgres, iterations, notify, message, rag, and many others in the database queries). The AgentConfigDefaults namespace is actively used inpackages/server/src/utils/agents.utils.tswith all property accesses working correctly. There are no functional issues with the current implementation.
config/agents/example.agent.json
Outdated
| "provider": "google-genai", | ||
| "model_name": "gemini-2.5-flash", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify model name and consider documentation for required environment variable.
The switch from OpenAI (GPT-4o) to Google GenAI (Gemini-2.5-Flash) aligns with the PR's goal to adopt new defaults. However, this represents a breaking change for users who may be using this example as a template—they will now need to set GEMINI_API_KEY instead of OPENAI_API_KEY.
Before merging, verify that:
gemini-2.5-flashis a valid current model name- Users are directed to configure the required
GEMINI_API_KEY(e.g., via comments or documentation)
🌐 Web query:
What is the correct Gemini model name in the Google GenAI API?
💡 Result:
Use the Gemini model ID string (e.g., "gemini-2.0-flash" or "gemini-2.5-pro") as the model name. The API also accepts full or partial resource names such as:
- models/gemini-2.0-flash
- publishers/google/models/gemini-2.0-flash
- projects/{project}/locations/{location}/publishers/google/models/gemini-2.0-flash
(Examples and accepted formats are documented by Google.) [1][2][3]
Sources:
[1] Google GenAI / @google/genai models docs.
[2] Gemini API — model name patterns (ai.google.dev).
[3] Vertex AI Generative Models (cloud.google.com).
Update model name to valid Gemini API identifier; current name gemini-2.5-flash is not recognized.
The model name gemini-2.5-flash does not match any valid Gemini model according to Google's documentation. Valid model identifiers include gemini-2.0-flash or gemini-2.5-pro. Update line 37 to use one of these valid model names. Additionally, users adopting this template will need to set GEMINI_API_KEY instead of OPENAI_API_KEY.
🤖 Prompt for AI Agents
In config/agents/example.agent.json around lines 36 to 37, the model_name value
"gemini-2.5-flash" is invalid for the Google Gemini provider; change it to a
valid identifier such as "gemini-2.0-flash" or "gemini-2.5-pro" and update
documentation/environment guidance so consumers of this template use
GEMINI_API_KEY (not OPENAI_API_KEY) when provider is "google-genai".
| AgentConfig, | ||
| MemoryStrategy, | ||
| } from '@common/agent/interfaces/agent.interface.js'; | ||
| import { Agent } from 'http'; |
There was a problem hiding this comment.
🧩 Analysis chain
Remove unused import.
The Agent import from Node's http module is not used anywhere in this file.
Apply this diff to remove the unused import:
-import { Agent } from 'http';Or verify whether this import is needed:
🏁 Script executed:
#!/bin/bash
# Search for usage of Agent identifier in this file
rg -n '\bAgent\b' packages/core/src/common/constant/agents.constants.tsLength of output: 133
Remove unused import Agent from 'http'.
The verification confirms that Agent is not used anywhere in the file as an actual code reference. The matches found are only in JSDoc comments and string literals (e.g., "Agent Selector" configuration names), not actual uses of the imported class.
-import { Agent } from 'http';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { Agent } from 'http'; |
🤖 Prompt for AI Agents
In packages/core/src/common/constant/agents.constants.ts around line 5, remove
the unused import "Agent" from 'http' because it is not referenced in the code
(only appears in JSDoc/comments/strings); update the import list to exclude
Agent so there are no unused imports and ensure linting/compilation warnings are
resolved.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/server/src/services/agent.service.ts (1)
38-160: Fix inconsistent error logging at line 160.The field name changes from
userRequest.requesttouserRequest.contentare applied consistently throughout the method at lines 38, 44, 110, 130, and 134. However, line 160 logs the entireuserRequestobject instead ofuserRequest.content, which is inconsistent with the error logging pattern used elsewhere (e.g., line 110).Apply this diff to fix the inconsistency:
error: { message: error.message, name: error.name, stack: error.stack, }, - request: userRequest, + request: userRequest.content, });packages/server/src/agents.storage.ts (1)
367-410: Remove dead code from initialization.The
modelInstance(lines 367-377) andagentConfigResolver(lines 379-410) are created but never used or stored. This appears to be orphaned code from the removedAgentSelectorinitialization that should have been cleaned up whengetAgentSelector()andgetModelFromUser()were removed.Apply this diff to remove the unused code:
} catch (error) { // Reset promise on failure so we can retry this.initializationPromise = null; throw error; } } /** * Perform the actual initialization logic * @private */ private async performInitialize(): Promise<void> { try { // Initialize global database configuration service DatabaseConfigService.getInstance().initialize(); // Wait for database service to be ready instead of polling await this.databaseService.onReady(); await DatabaseStorage.connect(); // Initialize Redis connection try { const redisClient = RedisClient.getInstance(); await redisClient.connect(); logger.log('Redis connected for agent storage'); } catch (error) { logger.error('Failed to initialize Redis connection:', error); throw error; } await this.init_agents_config(); this.initialized = true; } catch (error) { logger.error('Error during agent storage initialization:', error); this.initialized = false; throw error; } }Note: Remove lines 365-410 from the
initialize()method, keeping only the assignment and error handling forinitializationPromise.
♻️ Duplicate comments (1)
packages/core/src/common/constant/agents.constants.ts (1)
5-5: Remove unused import (duplicate issue).The
Agentimport from 'http' is still not used anywhere in this file, as confirmed by the previous review.Apply this diff to remove it:
-import { Agent } from 'http';
🧹 Nitpick comments (2)
docker-compose.yml (1)
145-183: Resolve duplicate service definitions.Both the old local build-based format (lines 145–165) and new image-based format (lines 167–183) for snak-frontend and snak-bff are commented out. This creates ambiguity about which is the intended target state and may confuse future maintainers.
Consider:
- Removing the old build-based definitions entirely if they're obsolete.
- Uncommenting one version as the active configuration, or adding a comment explaining which should be used for different deployment scenarios.
packages/core/src/common/constant/agents.constants.ts (1)
130-173: Prefer plain object export over namespace.TypeScript namespaces are considered legacy syntax. Modern TypeScript favors ES2015 module patterns.
As per coding guidelines.
Refactor to a plain object export:
-export namespace AgentConfigDefaults { - export const graph = { +export const AgentConfigDefaults = { + graph: { max_steps: 50, max_iterations: 50, max_retries: 3, execution_timeout_ms: 120000, max_token_usage: 16000, model: { provider: 'gemini', model_name: 'gemini-2.5-flash', temperature: 0.7, max_tokens: 8192, }, - }; + }, - export const memory = { + memory: { ltm_enabled: true, strategy: MemoryStrategy.HOLISTIC, size_limits: { short_term_memory_size: 10, max_insert_episodic_size: 20, max_insert_semantic_size: 20, max_retrieve_memory_size: 20, limit_before_summarization: 10000, }, thresholds: { insert_semantic_threshold: 0.8, insert_episodic_threshold: 0.75, retrieve_memory_threshold: 0.4, hitl_threshold: 0.85, }, timeouts: { retrieve_memory_timeout_ms: 20000, insert_memory_timeout_ms: 10000, }, - }; + }, - export const rag = { + rag: { enabled: false, top_k: 0, - }; + }, - export const mcp_servers = {}; -} + mcp_servers: {}, +} as const;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
config/agents/cairo.agent.json(1 hunks)config/agents/default.agent.json(1 hunks)config/agents/ethereum-rpc.agent.json(1 hunks)config/agents/example.agent.json(1 hunks)config/agents/memory.agent.json(1 hunks)config/agents/starknet-rpc.agent.json(1 hunks)config/agents/supervisor.agent.json(1 hunks)config/guards/default.guards.json(1 hunks)docker-compose.yml(3 hunks)packages/core/src/common/constant/agents.constants.ts(4 hunks)packages/core/src/config/guards/guardsSchema.ts(1 hunks)packages/core/src/index.ts(2 hunks)packages/core/src/services/agent-validation.service.ts(2 hunks)packages/database/src/queries/agents/queries.ts(1 hunks)packages/server/src/agents.storage.ts(3 hunks)packages/server/src/controllers/agents.controller.ts(3 hunks)packages/server/src/services/agent.service.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- config/agents/cairo.agent.json
- config/agents/example.agent.json
- packages/core/src/config/guards/guardsSchema.ts
- config/agents/starknet-rpc.agent.json
- packages/server/src/controllers/agents.controller.ts
- packages/core/src/services/agent-validation.service.ts
- config/agents/ethereum-rpc.agent.json
🧰 Additional context used
🧬 Code graph analysis (2)
packages/server/src/services/agent.service.ts (1)
packages/agent/src/shared/types/graph.type.ts (1)
UserRequest(84-87)
packages/server/src/agents.storage.ts (2)
packages/server/src/utils/agents.utils.ts (1)
initializeModels(16-53)packages/agent/src/agents/core/snakAgent.ts (1)
SnakAgent(33-434)
🪛 ESLint
packages/core/src/common/constant/agents.constants.ts
[error] 130-173: ES2015 module syntax is preferred over namespaces.
(@typescript-eslint/no-namespace)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Validate and Check
- GitHub Check: validate-build
🔇 Additional comments (15)
docker-compose.yml (2)
44-51: Security improvement: explicit Redis password requirement.The refactored redis command and healthcheck use
${REDIS_PASSWORD:?REDIS_PASSWORD is required}syntax to enforce the presence of the required environment variable with an explicit error message. This aligns well with the PR objective of improving environment security.However, ensure that:
- The
REDIS_PASSWORDenvironment variable is documented in.env.exampleor setup guides.- Deployment/CI pipelines properly inject this required variable before starting the container.
The YAML array formatting for command and healthcheck is also more robust than inline strings, as it avoids parsing issues with special characters.
Also applies to: 53-60
100-143: Clarify intent: authentication services disabled.Multiple authentication and security services (kc_postgres, keycloak, oauth2-proxy) are now commented out. The PR description mentions removing "everything related to StarkNet," but these services are unrelated to Starknet and appear to be core authentication infrastructure.
Please clarify:
- Is this intentional decom of these services, or accidental?
- Are there alternative authentication mechanisms in place?
- Should this be documented in migration or deployment guides if it represents a breaking change?
packages/server/src/services/agent.service.ts (1)
26-26: LGTM - Interface implementation removed as intended.The removal of the
IAgentServiceimplementation aligns with the PR's objective to clean up interfaces and remove dead code.config/agents/memory.agent.json (2)
52-56: Potential embedding model mismatch: OpenAI embedding with Gemini provider.The embedding model
text-embedding-ada-002is an OpenAI model, but the agent's primary model provider has been switched to Gemini (lines 25–26). Verify whether this is intentional (e.g., using OpenAI embeddings with Gemini as the generation model) or an oversight that should use a Gemini-compatible embedding model.If you intend to use a Gemini-compatible embedding model, consider switching to
text-embedding-004or another Gemini embedding option.
25-26: Model is valid and supported.The "gemini-2.5-flash" model is a valid Gemini model available via the API and Vertex AI (GA), confirming the model designation is current and correct.
packages/core/src/common/constant/agents.constants.ts (1)
31-32: LGTM! Model configuration corrected.The provider and model_name values have been properly aligned. Setting
provider: 'gemini'withmodel_name: 'gemini-2.5-flash'follows the correct pattern where the provider represents the service and the model_name specifies the exact model variant.Also applies to: 93-94
packages/server/src/agents.storage.ts (3)
519-524: LGTM!The model initialization correctly handles the null case returned by
initializeModels. Note thatinitializeModelslogs detailed error information before returning null, so the error thrown here provides a high-level message while detailed diagnostics are already logged.
590-593: LGTM!Consistent model initialization pattern with proper null handling.
630-631: LGTM!The
SnakAgentconstruction correctly uses the new single-argument constructor signature, passing onlyAgentConfigRuntimeas required.config/agents/supervisor.agent.json (1)
27-28: Supervisor agent correctly migrated to Gemini.Changes align with the guard constraints. Both
"gemini"provider and"gemini-2.5-flash"model are allowed perconfig/guards/default.guards.jsonlines 106–107.config/guards/default.guards.json (2)
105-110: All agent configurations have been successfully migrated to Gemini. The verification confirms that all seven agents (cairo,default,ethereum-rpc,example,memory,starknet-rpc,supervisor) now use thegeminiprovider withgemini-2.5-flashmodel, which is supported in the allowed models list.
105-110: Membership validation is properly enforced at runtime; no changes required.Verification confirms that membership checks ARE correctly implemented. While the guardsSchema itself contains only type definitions (
z.string(),z.array(z.string())), thevalidateModelConfigmethod inpackages/core/src/services/agent-validation.service.tsenforces the constraints at runtime:
- Line 281 validates
model.providerequalsallowedProvider(case-insensitive)- Line 291 validates
model.model_nameis in theallowedModelsarrayBoth checks throw descriptive errors on failure, ensuring strict compliance with the guard allowlist.
config/agents/default.agent.json (1)
43-44: Gemini provider migration verified across all agent configurations.All 7 agent files have been successfully migrated to the Gemini provider with the
gemini-2.5-flashmodel. Verification confirms:
- All agents use allowed provider
"gemini"and model"gemini-2.5-flash"(within the allowlist inconfig/guards/default.guards.json)- No legacy provider references (OpenAI, Anthropic, Claude) remain
GEMINI_API_KEYenvironment configuration is documented in README.mdMigration is complete and guard constraints are satisfied.
packages/database/src/queries/agents/queries.ts (1)
7-7: <!-- [scratchpad_start]
[task verify_import_1/1 verify_export_path][observations]
- Line 7 imports ModelConfig from '@snakagent/core' (confirmed)
- ModelConfig from agent.interface.ts has exact matching fields: provider, model_name, temperature, max_tokens
- Query result structure matches these fields exactly
- common/index.ts only re-exports from config.dto, message.dto, mcp.dto - NOT from agent.interface.ts
- agent.interface.ts location: packages/core/src/common/agent/interfaces/agent.interface.ts
[analysis]
- The import '@snakagent/core' is the package name, not a file path
- ModelConfig must be exported from the core package's main entry point
- agent.interface.ts has the correct structure for the query
- Need to verify the core package main entry point exports ModelConfig
[pending]
- Find packages/core main entry point (index.ts or package.json main/exports field)
- Verify ModelConfig is exported there
[actions]
- Search for packages/core/src/index.ts or packages/core/index.ts
- Check if agent.interface.ts ModelConfig is re-exported from main entry
packages/core/src/index.ts (1)
1-18: Export surface cleanup looks solid.
Thanks for centralizing the agent interfaces and DTO barrels here; the side-effect import also guarantees decorator metadata is ready for consumers. Nothing concerning on my end.
Clean
Changement
Summary by CodeRabbit
Configuration Changes
New Features
Improvements