Skip to content

Conversation

@0xhijo
Copy link
Member

@0xhijo 0xhijo commented Oct 28, 2025

Summary by CodeRabbit

  • Chores
    • Added internal logging statements for debugging purposes across the agent service architecture. These changes support development monitoring without affecting end-user functionality.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 28, 2025

Walkthrough

Added console.log debugging statements across three files to output tools information during initialization and model invocation. In mcp.ts, parseTools is now awaited in initializeConnections to ensure completion before proceeding. No changes to control flow logic or public interfaces.

Changes

Cohort / File(s) Summary
Debugging output additions
packages/agent/src/agents/graphs/core-graph/agent.graph.ts, packages/agent/src/agents/graphs/sub-graph/task-executor.graph.ts, packages/agent/src/services/mcp/src/mcp.ts
Added console.log statements to output tools list/array length and contents at initialization and invocation stages. In mcp.ts, parseTools call is now awaited in initializeConnections to ensure tool population completes before proceeding.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Focus areas: Verify that the await addition in mcp.ts::initializeConnections does not introduce unintended sequencing or performance implications; confirm logging statements are intended to be permanent or if they should be removed before production.

Poem

🐰 Hippity hop, let's debug with glee,
Console logs hopping wild and free!
Tools and tasks now loudly sing,
Await ensures the perfect timing! 🔍✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix mcp tools loading" is partially related to the changeset. The title accurately refers to a substantive change in the MCP module where parseTools is now awaited in initializeConnections, which does address the tools loading mechanism. However, the majority of the changes across all three files are console.log debugging statements rather than functional fixes. The title captures one real aspect of the changes but doesn't reflect that most of the changeset consists of logging additions.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/mcp-tool-calling

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
packages/agent/src/agents/graphs/sub-graph/task-executor.graph.ts (1)

117-117: Remove debugging console.log or replace with logger.

This console.log statement appears to be a debugging artifact from troubleshooting the MCP tools loading issue. Since the file already imports and uses logger throughout, either remove this line or replace it with a proper log statement if this information is needed for operational visibility.

Apply this diff to remove the debugging statement:

-    console.log(this.toolsList.length);

Or replace with proper logging if needed:

-    console.log(this.toolsList.length);
+    logger.debug(`[Executor] Tools available: ${this.toolsList.length}`);
packages/agent/src/agents/graphs/core-graph/agent.graph.ts (1)

151-151: Remove duplicate debugging console.log statements or replace with logger.

These console.log statements appear to be debugging artifacts. The same tools list length is logged twice—once after initialization (line 151) and again at the start of workflow building (line 406). Since the file already imports and uses logger throughout, either remove these lines or consolidate into a single proper log statement if operational visibility is needed.

Apply this diff to remove the debugging statements:

-      console.log(`Tools list length: ${this.toolsList.length}`);
       // Initialize RAG agent if enabled
-    console.log(`Tools list length: ${this.toolsList.length}`);
     const memory = new MemoryGraph(

Or replace with proper logging if needed:

-      console.log(`Tools list length: ${this.toolsList.length}`);
+      logger.info(`[Agent] Initialized ${this.toolsList.length} tools`);
-    console.log(`Tools list length: ${this.toolsList.length}`);
+    logger.debug(`[Agent] Building workflow with ${this.toolsList.length} tools`);

Also applies to: 406-406

packages/agent/src/services/mcp/src/mcp.ts (1)

64-64: Remove debugging console.log or replace with existing logger.

This console.log statement appears to be a debugging artifact. Since the file already imports and uses logger throughout, and line 65 already logs the tools count, this additional console.log is redundant.

Apply this diff to remove the debugging statement:

       const tools = await this.client.getTools();
       this.tools = tools;
-      console.log(tools);
       logger.info(`Parsed ${tools.length} tools from MCP servers`);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1eb2298 and d57c853.

📒 Files selected for processing (3)
  • packages/agent/src/agents/graphs/core-graph/agent.graph.ts (2 hunks)
  • packages/agent/src/agents/graphs/sub-graph/task-executor.graph.ts (1 hunks)
  • packages/agent/src/services/mcp/src/mcp.ts (2 hunks)
⏰ 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: Build Check
  • GitHub Check: validate-build
🔇 Additional comments (1)
packages/agent/src/services/mcp/src/mcp.ts (1)

82-82: Excellent fix for the race condition.

Adding await ensures that parseTools() completes and populates this.tools before initializeConnections() returns. This was the root cause of the tools loading issue—without the await, the tools array could be empty when accessed downstream.

@0xhijo 0xhijo closed this Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants