Skip to content

Conversation

@MaheshtheDev
Copy link
Member

No description provided.

@MaheshtheDev MaheshtheDev marked this pull request as ready for review January 17, 2026 00:36
@graphite-app graphite-app bot requested a review from Dhravya January 17, 2026 00:36
Copy link
Member Author


How to use the Graphite Merge Queue

Add the label Main to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 17, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
supermemory-app acdb422 Jan 17 2026, 01:03 AM

@claude
Copy link

claude bot commented Jan 17, 2026

PR Review: Add MCP Projects Aware Tool on Every Init

Summary

This PR adds a new listProjects tool to the MCP server and fetches available projects on initialization to dynamically update tool schemas with project information. It also upgrades the @modelcontextprotocol/sdk and agents dependencies.


✅ Positive Changes

  1. Better UX: The dynamic schema descriptions that include available projects (apps/mcp/src/server.ts:66-67,79-80,87-88) help AI assistants understand which projects are valid, reducing errors.

  2. New listProjects tool: Provides a way for clients to discover available projects programmatically (apps/mcp/src/server.ts:184-239).

  3. Proper initialization timing: Projects are fetched during init() at server.ts:53, ensuring the schema descriptions are accurate from the start.

  4. Error handling: The refreshContainerTags() method properly catches and logs errors without crashing initialization (server.ts:610-617).


🔍 Issues & Recommendations

1. Async/Await Issue in Constructor Context ⚠️

Location: apps/mcp/src/server.ts:53

async init() {
    // ...
    await this.refreshContainerTags() // Fetch available projects for schema descriptions

The await here is inside init(), which is async. However, the tool registrations (server.registerTool) happen synchronously right after this, using containerTagDescription at line 55.

Problem: If refreshContainerTags() takes time, the description might be generated before projects are fetched, defeating the purpose.

Recommendation: Consider whether this is truly guaranteed to complete before schema registration, or if you need to restructure the initialization flow.


2. Silent Failure in Project Fetching

Location: apps/mcp/src/server.ts:610-617

private async refreshContainerTags(): Promise<void> {
    try {
        const client = this.getClient()
        this.cachedContainerTags = await client.getProjects()
    } catch (error) {
        console.error("Failed to fetch container tags:", error)
    }
}

If getProjects() fails (network error, auth failure), the cachedContainerTags remains empty [], and the schema descriptions won't include project information. This is silently handled.

Recommendation: Consider whether this should:

  • Log a warning to the user
  • Set a flag indicating projects couldn't be loaded
  • Retry with exponential backoff

For an MCP server, graceful degradation is acceptable, but users won't know why projects aren't appearing in tool descriptions.


3. Dependency Version Jumps

Location: apps/mcp/package.json:12-13

  • @modelcontextprotocol/sdk: ^1.12.1^1.25.2 (13 minor versions)
  • agents: ^0.2.32^0.3.5 (breaking change from 0.2 → 0.3)

Concern: The agents package jumped from 0.2.x to 0.3.x, which could introduce breaking changes. The PR title doesn't mention dependency upgrades.

Recommendation:

  • Verify the agents 0.3.x upgrade doesn't introduce breaking changes to the McpAgent API
  • Consider splitting dependency upgrades into a separate PR for easier rollback
  • Document any behavioral changes from these upgrades

4. Type Safety - @ts-expect-error Suppressions

Locations: apps/mcp/src/server.ts:198, 287

// @ts-expect-error - zod type inference issue with MCP SDK
async (args: { refresh?: boolean }) => {

These suppressions were added for the new code. While the comment suggests it's a Zod/MCP SDK inference issue, it would be better to understand the root cause.

Recommendation: File an issue with the MCP SDK about the Zod type inference, or use proper type assertions instead of error suppression.


5. Import Order (Minor Style)

Location: apps/mcp/src/index.ts:1-2

-import { Hono } from "hono"
 import { cors } from "hono/cors"
+import { Hono } from "hono"

While Biome handles this automatically, the import order changed. Ensure your Biome config enforces consistent ordering.


📋 Testing Recommendations

Since apps/mcp has no test setup, manual testing is critical:

  1. Test project fetching on init: Verify listProjects returns correct data on first connection
  2. Test error scenarios: What happens if API is down during init? Do tools still register correctly?
  3. Test with no projects: Verify the fallback description works when user has no projects
  4. Test containerTag parameter: Ensure the dynamic descriptions actually help the AI assistant choose correct projects

🚀 Summary

Approve with minor concerns. The feature is well-implemented, but:

  • Verify the async initialization timing issue
  • Consider better error visibility for project fetch failures
  • Confirm agents@0.3.x doesn't introduce breaking changes

The core functionality looks solid and the error handling is defensive. Good job maintaining backward compatibility by keeping schemas optional!

@Dhravya Dhravya self-requested a review January 17, 2026 00:56
@graphite-app
Copy link

graphite-app bot commented Jan 17, 2026

Merge activity

@graphite-app graphite-app bot force-pushed the 01-16-add_mcp_projects_aware_tool_on_every_init branch from f284d55 to acdb422 Compare January 17, 2026 00:57
@graphite-app graphite-app bot merged commit acdb422 into main Jan 17, 2026
4 of 6 checks passed
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.

3 participants