Skip to content

Conversation

@amikofalvy
Copy link
Collaborator

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Jan 26, 2026

🦋 Changeset detected

Latest commit: 5d40c24

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@inkeep/agents-mcp Major
@inkeep/agents-api Major
@inkeep/agents-manage-ui Major
@inkeep/agents-cli Major
@inkeep/agents-core Major
@inkeep/agents-manage-mcp Major
@inkeep/agents-sdk Major
@inkeep/ai-sdk-provider Major
@inkeep/create-agents Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Jan 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
agents-api Ready Ready Preview, Comment Jan 27, 2026 0:54am
agents-docs Ready Ready Preview, Comment Jan 27, 2026 0:54am
agents-manage-ui Ready Ready Preview, Comment Jan 27, 2026 0:54am

Request Review

@claude
Copy link

claude bot commented Jan 26, 2026

Claude finished @amikofalvy's task —— View job


Comprehensive Code Review

Summary

This PR introduces a new MCP (Model Context Protocol) Server package that exposes the Inkeep Agents API, allowing Claude Desktop, Cursor, VS Code, and other MCP clients to interact with the Inkeep agent framework. The implementation is primarily code-generated using Speakeasy from OpenAPI specifications.

Architectural Review

Strengths

Clean Generated Code Architecture:

  • Well-structured TypeScript with proper type safety and error handling
  • Good separation between generated SDK code (src/funcs/, src/lib/) and integration points
  • Comprehensive error type hierarchy with APIError, SDKValidationError, etc.
  • Proper use of functional programming patterns with Result<T, E> types

MCP Integration Design:

  • Clean integration pattern in agents-api/src/domains/mcp/routes/mcp.ts
  • Smart header forwarding with browser compatibility (x-forwarded-cookiecookie)
  • Proper dependency injection of SDK instances with authentication hooks

Tooling and CI/CD:

  • Automated SDK generation via GitHub Actions with Speakeasy
  • Proper turbo.json integration with custom build outputs
  • Multi-platform installation support (Claude Desktop, Cursor, VS Code, Windsurf)

Critical Issues

1. Missing Changeset (High Priority)
As noted by the changeset-bot, this PR introduces a new published package but lacks a changeset. According to the repository's AGENTS.md:

pnpm bump patch --pkg agents-mcp "Add MCP server for Inkeep Agents API"

The package name should be agents-mcp to match the existing naming pattern in line 43 of AGENTS.md.

2. No Test Coverage (High Priority)
The new package has zero test coverage:

  • No tests in packages/agents-mcp/src/__tests__/
  • No integration tests for the MCP endpoint in agents-api/src/domains/mcp/routes/mcp.ts
  • According to AGENTS.md, "ALL new work MUST include" unit tests - this is a mandatory requirement

3. Missing Documentation (Medium Priority)

  • No documentation in agents-docs/ explaining MCP usage patterns
  • The auto-generated README is Speakeasy boilerplate and doesn't explain integration with the existing system
  • Missing examples of how to use the MCP server with actual Inkeep agents

⚠️ Areas for Consideration

4. Authentication and Security Patterns
The header forwarding logic in mcp.ts:33-35 is clever but potentially fragile:

if (headersToForward['x-forwarded-cookie'] && !headersToForward.cookie) {
  headersToForward.cookie = headersToForward['x-forwarded-cookie'];
}

Consider: What happens if both headers are present? Should there be validation or precedence rules?

5. Error Handling and Logging

const noOpLogger = createConsoleLogger('error');

Using error-level logging only may hide important debugging information. Consider making log level configurable.

6. SDK Type Safety
The cast to any in mcp.ts:44 suggests potential type mismatches:

return new InkeepAgentsCore({
  serverURL: env.INKEEP_AGENTS_API_URL,
  hooks,
} as any);

This could hide type safety issues and should be resolved with proper typing.

7. Package Naming Inconsistency
The package name @inkeep/agents-mcp doesn't follow the repository's established pattern of @inkeep/agents-* packages. Consider @inkeep/agents-mcp or alignment with existing naming in AGENTS.md line 43.

💡 System Design Considerations

8. MCP Function Scope and Organization
With 100+ generated functions, users may find it overwhelming. Consider:

  • Grouping functions by domain (agents, evaluations, credentials, etc.)
  • Implementing function discovery/capabilities that match user permissions
  • Providing usage examples for common workflows

9. Performance and Scalability

  • The MCP server creates a new SDK instance per request - consider connection pooling
  • Large function sets may impact MCP client performance during capability discovery
  • Consider implementing streaming for large result sets

10. Future Maintainability

  • The DO NOT EDIT warnings are clear, but consider documenting the regeneration workflow
  • Speakeasy config in .speakeasy/gen.yaml lacks repository-specific customization
  • Missing integration with existing build/lint/test workflows beyond turbo.json

Recommendations

Immediate Action Items (Must Fix):

  1. Add changeset: pnpm bump patch --pkg agents-mcp "Add MCP server for Inkeep Agents API"
  2. Add test coverage: Create tests for both the generated SDK and MCP integration
  3. Fix type safety: Resolve the as any cast in the MCP integration

Follow-up Improvements:

  1. Documentation: Add comprehensive MCP usage docs to agents-docs/
  2. Error handling: Make logging configurable and improve error propagation
  3. Function organization: Consider grouping or filtering MCP functions by user context
  4. Performance: Evaluate SDK instance creation and connection patterns

System Architecture Questions:

  • How should MCP function permissions align with the existing agent RBAC system?
  • Should MCP servers be tenant-scoped or support multi-tenancy?
  • What's the strategy for handling breaking API changes in generated SDKs?

Verdict

This is a well-architected foundation for MCP integration with good separation of concerns and proper TypeScript patterns. However, it cannot be merged until the mandatory requirements (changeset, tests) are addressed. The generated code quality is high, but the integration points need more careful consideration of type safety and error handling.

The PR demonstrates good understanding of both MCP patterns and the existing Inkeep architecture. With the critical issues resolved, this will provide valuable functionality for developers wanting to interact with Inkeep agents through MCP-compatible tools.

The agents-api package depends on @inkeep/agents-mcp, so it needs to be
built before running the integration tests in CI.
Co-authored-by: Dimitri POSTOLOV <dmytropostolov@gmail.com>
Use pnpm turbo instead of bare turbo command since turbo is a devDependency, not globally installed.
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