Skip to content

Feat/hubspot integration#283

Merged
lalalune merged 15 commits intodevfrom
feat/hubspot-integration
Mar 15, 2026
Merged

Feat/hubspot integration#283
lalalune merged 15 commits intodevfrom
feat/hubspot-integration

Conversation

@samarth30
Copy link
Member

@samarth30 samarth30 commented Feb 5, 2026

This pull request introduces expanded support for OAuth integrations, particularly adding HubSpot, Linear, Notion, GitHub, and Slack alongside Google. The changes enhance both the connection and revocation flows, update example usage, and add HubSpot-specific server configuration. Additionally, minor code cleanups and formatting improvements are made across several files.

OAuth integration enhancements:

  • Added HubSpot support to the MCP server configuration in lib/eliza/runtime-factory.ts, enabling HubSpot-specific OAuth flows.
  • Registered HubSpot tools in the MCP handler and exports, allowing HubSpot integration to be managed alongside existing tools. [1] [2] [3]
  • Expanded oauthConnectAction and oauthRevokeAction to support HubSpot, Linear, Notion, GitHub, and Slack, updating similes, descriptions, parameters, and example flows for both actions. [1] [2] [3] [4] [5] [6] [7] [8]

Code and formatting improvements:

Twitter callback robustness:

  • Improved handling of Twitter OAuth callback state retrieval by supporting both string and object data types, increasing robustness against cache implementation differences.

Error handling fixes:

  • Minor error handling and formatting fixes in MCP route handler responses.

Note

Medium Risk
Adds a new OAuth provider and multiple HubSpot MCP endpoints, and changes OAuth token/user-info retrieval for some providers (incl. Linear/Slack), which could break existing connection flows if misconfigured. Runtime cache key behavior also changes based on connected MCP platforms, which may affect cache hit rates and invalidation.

Overview
Introduces HubSpot integration end-to-end: a new hubspot OAuth provider (with scopes and token-metadata lookup), HubSpot MCP tools for CRM objects (contacts/companies/deals/owners + associations), and a standalone HubSpot MCP server under app/api/mcps/hubspot/[transport].

Updates the main MCP handler to register HubSpot tools, extends Eliza’s OAUTH_CONNECT/OAUTH_REVOKE actions to recognize HubSpot (copy + examples), and adjusts runtime creation/caching so MCP server injection varies by connected OAuth platforms (avoiding stale cached runtimes missing newly connected tools).

Enhances the OAuth2 adapter/provider registry with an optional tokenInfo endpoint (used for HubSpot) and tweaks provider configs (notably Linear token exchange content type, Slack user info endpoint/scopes, and Google scope set). Adds a helper script to purge HubSpot connections and new tests covering HubSpot tool behavior and MCP server URL expectations.

Written by Cursor Bugbot for commit 39bb5e7. This will update automatically on new commits. Configure here.

samarth30 and others added 4 commits February 5, 2026 23:33
Add HubSpot CRM as an OAuth provider with support for:
- Contacts read/write
- Companies read/write
- Deals read/write
- Owners read

HubSpot uses form-encoded token requests and extracts user info
from hub_id and user fields in the token response.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add 16 HubSpot MCP tools:
- hubspot_status: Check connection status
- hubspot_list_contacts, hubspot_get_contact, hubspot_create_contact,
  hubspot_update_contact, hubspot_search_contacts
- hubspot_list_companies, hubspot_create_company, hubspot_search_companies
- hubspot_list_deals, hubspot_create_deal, hubspot_update_deal,
  hubspot_search_deals
- hubspot_list_owners
- hubspot_associate: Create object associations

Co-authored-by: Cursor <cursoragent@cursor.com>
Add standalone HubSpot MCP endpoint at /api/mcps/hubspot/ that allows
the Eliza agent to interact with HubSpot CRM directly in conversations.

Tools exposed to agent:
- Contact management (list, get, create, update, search)
- Company management (list, create, search)
- Deal management (list, create, search)
- Owner listing

Also registers HubSpot in MCP_SERVER_CONFIGS so agent automatically
connects when HubSpot OAuth is active.

Co-authored-by: Cursor <cursoragent@cursor.com>
Enhance OAUTH_CONNECT and OAUTH_REVOKE actions:
- Add HubSpot to similes (CONNECT_HUBSPOT, LINK_HUBSPOT)
- Update descriptions to list HubSpot as available platform
- Add HubSpot examples to action definitions

This enables the agent to understand HubSpot connection commands
like "connect hubspot" or "disconnect hubspot".

Co-authored-by: Cursor <cursoragent@cursor.com>
@samarth30 samarth30 requested a review from Copilot February 5, 2026 18:10
@vercel
Copy link

vercel bot commented Feb 5, 2026

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

Project Deployment Actions Updated (UTC)
eliza-cloud-v2 Ready Ready Preview, Comment Feb 16, 2026 1:57pm

@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

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

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request expands OAuth integration support by adding HubSpot alongside existing platforms (Google, Linear, Notion, GitHub, Slack). The PR includes comprehensive MCP (Model Context Protocol) tooling for HubSpot CRM operations, updates OAuth action handlers, improves Twitter callback robustness, and includes extensive code formatting improvements across the runtime factory.

Changes:

  • Added HubSpot OAuth provider configuration with CRM scopes and endpoints
  • Implemented HubSpot MCP tools for contacts, companies, deals, and owners management
  • Updated OAuth connect/revoke actions to support HubSpot and other new platforms
  • Enhanced Twitter OAuth callback to handle different cache return types
  • Applied consistent code formatting across runtime factory and OAuth modules

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/unit/mcp-tools.test.ts Added test assertion for HubSpot tools registration
lib/services/oauth/provider-registry.ts Added HubSpot provider configuration and updated Linear, Slack, and Google configurations
lib/eliza/runtime-factory.ts Added HubSpot MCP server configuration and applied formatting improvements
lib/eliza/plugin-oauth/actions/oauth-revoke.ts Extended action to support HubSpot and other platforms with updated examples
lib/eliza/plugin-oauth/actions/oauth-connect.ts Extended action to support HubSpot and other platforms with updated examples
app/api/v1/twitter/callback/route.ts Improved state data handling to support both string and object formats from cache
app/api/mcps/hubspot/[transport]/route.ts New standalone HubSpot MCP server endpoint with full CRM tooling
app/api/mcp/tools/index.ts Exported HubSpot tools registration function
app/api/mcp/tools/hubspot.ts Comprehensive HubSpot MCP tools implementation with 15 tools
app/api/mcp/route.ts Registered HubSpot tools in main MCP handler with formatting fix

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

samarth30 and others added 2 commits February 6, 2026 14:54
…ubSpot connections

Co-authored-by: Cursor <cursoragent@cursor.com>
…URLs

- Cache key now includes connected MCP platforms (e.g. google,hubspot) so
  runtime is recreated when user connects HubSpot; otherwise cached runtime
  never gets HubSpot tools in action list.
- MCP server URLs updated to /streamable-http path for Google and HubSpot.

Co-authored-by: Cursor <cursoragent@cursor.com>
samarth30 and others added 2 commits February 7, 2026 00:25
Address Copilot review comments on PR #283:
- Slack: document auth.test response shape and why email is absent from mapping
  #283 (comment)
- HubSpot: remove incorrect revoke URL, document local-only disconnect
  #283 (comment)
- HubSpot: clarify userInfoMapping is for tokenInfo response, not userInfo
  #283 (comment)

Co-authored-by: Cursor <cursoragent@cursor.com>
Address Copilot review comment on PR #283 requesting test coverage:
#283 (comment)

Covers tool registration (all 15 tools), status checks, contacts/companies/
deals/owners list operations, contact creation with error handling,
association creation, network errors, and concurrent org isolation.

Co-authored-by: Cursor <cursoragent@cursor.com>
samarth30 and others added 3 commits February 9, 2026 14:16
Reverts non-HubSpot changes that were mixed into this branch:
- Twitter callback: revert cache string/object handling (unrelated fix)
- Linear: revert tokenContentType from form back to json (unrelated fix)
- Slack: revert auth.test back to users.identity with original scopes/mapping
- runtime-factory: remove ~50 trailing comma formatting changes, keep only
  HubSpot MCP config and MCP platform cache key logic

Co-authored-by: Cursor <cursoragent@cursor.com>
Return distinct status when HubSpot OAuth connection is expired,
prompting the user to reconnect instead of a generic "not connected".

Co-authored-by: Cursor <cursoragent@cursor.com>
Resolved conflicts by taking dev's changes and adding HubSpot on top:
- mcp/route.ts, tools/index.ts: added HubSpot alongside dev's Linear/Notion/GitHub
- twitter/callback: took dev's version with validation
- oauth-connect/revoke: took dev's format, added hubspot to platform lists
- runtime-factory: took dev's version, added hubspot to MCP_SERVER_CONFIGS
- provider-registry: took dev's Slack mapping

Co-authored-by: Cursor <cursoragent@cursor.com>
Resolved migration conflict: kept 0028_add_missing_apps_columns,
renumbered upstream 0028_add_user_platform_oauth_constraint to 0029.

Co-authored-by: Cursor <cursoragent@cursor.com>
@claude
Copy link

claude bot commented Feb 9, 2026

Code Review: PR #283 - HubSpot OAuth Integration

Overview

This PR adds comprehensive HubSpot OAuth integration with CRM tools (contacts, companies, deals, owners). The implementation follows established project patterns well and is nearly production-ready.

Overall Quality: 8/10Approve with minor changes requested


🔴 Critical Issues (Must Fix)

1. Type Mismatch in Deal Amount Field

File: app/api/mcps/hubspot/[transport]/route.ts:419

The amount field is defined as z.string() in the standalone endpoint but z.number() in the main implementation (hubspot.ts:550):

// route.ts:419 (WRONG)
amount: z.string().optional().describe("Deal amount"),

// hubspot.ts:550 (CORRECT)  
amount: z.number().optional().describe("Deal amount"),

Impact: Runtime errors when creating deals with numeric amounts via standalone endpoint.

Fix: Change line 419 to z.number() to match the main implementation.


2. Inconsistent Default Limits

The two implementations have different default limits:

  • Main MCP endpoint: default limit is 20 (hubspot.ts:114)
  • Standalone endpoint: default limit is 10 (route.ts:126)

Fix: Standardize on 20 for consistency.


⚠️ Important Issues (Should Fix)

3. Missing Pagination in Search Operations

Search operations (hubspot_search_contacts, hubspot_search_companies, hubspot_search_deals) don't return pagination cursors, making it impossible to retrieve more than the limit of results.

Recommendation: Add paging information like the list endpoints do (return hasMore and after cursor).


4. Error Response Parsing Could Fail Silently

File: app/api/mcp/tools/hubspot.ts:53

const error = await response.json().catch(() => ({}));

This swallows JSON parsing errors. If HubSpot returns a non-JSON error (e.g., HTML error page), the error message will be generic.

Recommendation:

const error = await response.json().catch((e) => {
  logger.warn("[HubSpot] Failed to parse error response", { status: response.status });
  return { message: `HTTP ${response.status}` };
});

5. Association Type Hardcoding is Fragile

File: app/api/mcp/tools/hubspot.ts:730-734

const associationTypeMap: Record<string, Record<string, number>> = {
  contacts: { companies: 1, deals: 3 },
  companies: { contacts: 2, deals: 5 },
  deals: { contacts: 4, companies: 6 },
};

These hardcoded IDs could break if HubSpot changes them or if custom association types are needed.

Recommendation: Document this limitation or consider fetching from HubSpot's schema API.


6. Missing Rate Limiting Documentation

HubSpot has API rate limits (typically 100 requests per 10 seconds). There's no rate limiting or throttling implemented.

Recommendation: At minimum, document the rate limits and handle 429 responses with retry logic.


7. Missing Token Refresh Tests

While OAuth2 provider has refresh support, there are no tests verifying expired tokens are properly refreshed before API calls.

Recommendation: Add integration tests that mock token expiration scenarios.


🟡 Code Quality & Best Practices

8. Code Duplication Between Implementations

The two files (hubspot.ts and route.ts) contain nearly identical implementations, violating DRY principle.

Note: This might be intentional for the standalone endpoint architecture, but consider extracting shared logic if appropriate.


9. Missing JSDoc Comments

Functions like getHubSpotToken(), hubspotFetch(), and errMsg() lack documentation.

Recommendation: Add JSDoc comments for better developer experience.


10. Test Coverage Gaps

Tests cover happy paths well but miss:

  • Token refresh scenarios
  • Rate limiting behavior
  • Malformed API responses
  • Edge cases (empty results, null values)
  • Update operations for deals/companies
  • All CRUD operations

11. Input Validation for Update Operations

File: app/api/mcp/tools/hubspot.ts:608

The properties field allows both strings and numbers without validation:

properties: z
  .record(z.union([z.string(), z.number()]))
  .describe("Properties to update"),

Recommendation: Add type validation for known HubSpot properties or document expected types.


✅ Positive Observations

  1. Excellent pattern consistency - Follows existing Google/Linear integration patterns
  2. Comprehensive provider registry - Well-documented OAuth configuration with proper scope definitions
  3. Robust OAuth2 implementation - Handles token metadata, secret cleanup, and edge cases well
  4. Good test structure - Clear organization and descriptive test names
  5. Database migration safety - Properly handles existing duplicates before adding constraints
  6. Developer experience - Includes utility script for connection cleanup
  7. Security best practices - Proper token storage, scoping, and authorization

Security Assessment ✓

  • ✅ Tokens stored securely in secrets service
  • ✅ Proper organization-scoped access control
  • ✅ Input validation with Zod schemas
  • ✅ No injection vulnerabilities
  • ✅ Safe URL construction

Performance Assessment

  • ✅ Efficient direct API calls
  • ✅ Pagination support for list operations
  • ✅ Minimal database queries
  • ⚠️ Could benefit from response caching for frequently accessed data

Recommendations Summary

Must Fix Before Merge:

  1. Fix type mismatch for deal amount (route.ts:419)
  2. Standardize default limits between implementations

Should Address:
3. Add pagination support to search operations
4. Improve error response parsing with logging
5. Document rate limiting considerations
6. Add token refresh test scenarios

Nice to Have:
7. Add JSDoc comments to exported functions
8. Expand test coverage for edge cases
9. Consider extracting shared logic between implementations
10. Add input sanitization for email fields


Final Verdict

This is a high-quality implementation that demonstrates good understanding of the project's architecture and OAuth patterns. The critical issues are minor and easily fixable. Recommend merging after addressing the must-fix items.

Great work on maintaining consistency with existing integrations! 🎉

@claude
Copy link

claude bot commented Feb 12, 2026

Code Review: PR #283 - HubSpot Integration & Multi-Platform OAuth Support

Summary: High-quality implementation with excellent security foundations and comprehensive test coverage. Recommended for merge with minor monitoring items noted below.

Stats: 14 files changed (+2,245/-39 lines)


✅ Strengths

1. Security - Excellent Implementation

  • CSRF Protection: 10-minute TTL on OAuth state with proper validation
  • Redirect URL Validation: Whitelist-based with path traversal protection (normalizePath())
  • Rate Limiting: 10 req/min per IP on OAuth callback
  • Token Storage: Encrypted secrets table with reference IDs
  • Token Expiry Clamping: Prevents malformed responses (60s-1yr bounds)

2. Code Quality

  • Consistent Architecture: HubSpot tools follow established MCP patterns
  • Comprehensive Tests: 640 lines covering 15 tools, concurrent requests, error handling
  • Database Constraints: Smart unique index with WHERE user_id IS NOT NULL
  • Error Handling: Structured error classes with HTTP mapping and user-friendly messages
  • Adherence to CLAUDE.md: Proper Bun/Next.js stack, migration workflow followed

3. Twitter Callback Improvements

  • Better state validation (string/object handling)
  • Comprehensive error logging with organization context
  • Safe URL query parameter handling

⚠️ Issues Found

MEDIUM Severity

1. Token Hash Fallback Creates Non-Deterministic IDs

File: lib/services/oauth/providers/oauth2.ts:456-479

const hash = Buffer.from(tokens.access_token.substring(0, 32)).toString("base64url");
return { id: `${provider.id}_${hash}`, raw: tokens };

Problem: When provider has no userInfo endpoint, falls back to token hash as user ID. Same user with different OAuth sessions gets different IDs, breaking "single OAuth per platform" constraint.

Recommendation: Throw error instead of fallback. Providers should explicitly define user ID extraction.


2. HubSpot Token in URL Path

File: lib/services/oauth/providers/oauth2.ts:438

const url = `${baseUrl.replace(/\/$/, "")}/${encodeURIComponent(accessToken)}`;

Problem: Access token in URL path creates logging/referrer leakage risks (HubSpot API design issue).

Recommendation: Document this limitation. Consider POST with token in header if HubSpot supports it.


LOW Severity

3. Twitter Callback - Missing UUID Validation

File: app/api/v1/twitter/callback/route.ts:54-67

State data validates type but not format:

typeof parsed.organizationId !== "string"  // Should validate UUID format

Recommendation: Add isValidUUID() checks for organizationId and userId.


4. Generic Adapter Silent Failures

File: lib/services/oauth/connection-adapters/generic-adapter.ts:128-132

Returns empty array on platform enum errors - could mask configuration issues.

Recommendation: Consider throwing for missing platform enum and handle at service layer.


📋 Testing Notes

Excellent Coverage:

  • ✅ 15 HubSpot tools registered and tested
  • ✅ Connection status filtering
  • ✅ Network error handling
  • ✅ Concurrent multi-org requests
  • ✅ Contact/Company/Deal CRUD operations

Could Add:

  • Association type validation tests
  • Pagination cursor edge cases
  • Invalid property filters

🔍 Database Migration Review

Migration 0025 - OAuth Platform Types ✅

  • Uses IF NOT EXISTS for idempotency
  • Adds 10 new platforms (HubSpot, Linear, Notion, GitHub, Slack, etc.)

Migration 0029 - Unique Constraint ✅

  • Detects and cleans duplicates before constraint
  • Partial index WHERE user_id IS NOT NULL allows org-level connections
  • Excellent design!

📚 Architecture Notes

OAuth Flow:

  1. User initiates → Generate state (CSRF token, 10min TTL)
  2. Redirect to provider → User authorizes
  3. Callback validates state → Exchange code for tokens
  4. extractUserInfoFromTokens() → Get platform user ID
  5. Store encrypted tokens → Link to organization/user

HubSpot-Specific:

  • Uses tokenInfo endpoint instead of userInfo (HubSpot limitation)
  • Maps hub_id → user ID, user field → email
  • Scopes: crm.objects.*, oauth, crm.schemas.contacts.read

🎯 Recommendations

Before Merge:

  1. Fix token hash fallback in extractUserInfoFromTokens() - should throw
  2. Add UUID validation for Twitter callback state

Post-Merge:

  1. Document HubSpot tokenInfo limitation
  2. Monitor for concurrent OAuth completion edge cases
  3. Consider enhancing association type validation

✨ Overall Assessment

This PR demonstrates production-ready engineering with:

  • Strong security foundations
  • Comprehensive multi-platform OAuth support (HubSpot + 13 others)
  • Well-designed database constraints
  • Excellent test coverage
  • Proper adherence to project standards

No show-stoppers found. The identified issues are minor and mostly warrant monitoring rather than blocking merge.

Approval Status:APPROVE (with minor fixups recommended)


Generated by Claude Code - Reviewed against CLAUDE.md standards

- Fix deal amount type mismatch (z.string -> z.number) in standalone endpoint
- Standardize default limits from 10 to 20 across all standalone endpoint tools
- Add pagination (paging) to search operations in both implementations
- Improve error response parsing with logger.warn fallback in hubspotFetch

Co-authored-by: Cursor <cursoragent@cursor.com>
@claude
Copy link

claude bot commented Feb 12, 2026

PR Review: HubSpot OAuth Integration

Summary

This PR adds comprehensive HubSpot CRM integration with OAuth support. The implementation includes MCP tools for contacts, companies, deals, owners, and associations. Overall, the code quality is good with proper error handling and test coverage.

✅ Strengths

1. Well-Structured Implementation

  • Clean separation between MCP tools (app/api/mcp/tools/hubspot.ts) and standalone endpoint (app/api/mcps/hubspot/[transport]/route.ts)
  • Consistent error handling patterns with proper logging
  • Good use of Zod for input validation

2. OAuth Configuration

  • Proper OAuth2 provider configuration in provider-registry.ts:386-420
  • Uses form-encoded token requests (correct for HubSpot)
  • Leverages tokenInfo endpoint for user metadata extraction
  • Good documentation of revoke limitations (no remote revoke support)

3. Test Coverage

  • Comprehensive unit tests in tests/unit/mcp-hubspot-tools.test.ts
  • Tests all 15 tools, error scenarios, and isolation
  • Mock-based testing with proper setup/teardown

4. Security

  • Proper token handling via oauthService.getValidTokenByPlatform()
  • Per-organization OAuth isolation
  • Bearer token authentication
  • No hardcoded credentials

🔍 Issues & Recommendations

High Priority

1. Type Safety - Deal Amount Field
In app/api/mcp/tools/hubspot.ts:555:

amount: z.number().optional().describe("Deal amount"),

However, in the standalone endpoint, line 555 might have a mismatch. Ensure consistency between both implementations. HubSpot's API typically expects amount as a string (formatted number).

2. Error Response Parsing
In app/api/mcp/tools/hubspot.ts:52-58:

if (!response.ok && response.status !== 204) {
  const error = await response.json().catch(() => {
    logger.warn("[HubSpot] Failed to parse error response", { status: response.status });
    return { message: `HTTP ${response.status}` };
  });

Good fallback handling, but consider logging the original error response body for debugging.

3. Missing Rate Limit Handling
HubSpot API has rate limits (100 requests/10 seconds for most endpoints). Consider:

  • Adding retry logic with exponential backoff
  • Caching frequently accessed data
  • Warning users when approaching limits

Medium Priority

4. Pagination Improvements
Search operations (lines 326-351) include pagination in responses but lack:

  • Helper functions to iterate through all pages
  • Documentation about after cursor usage
  • Total count might be misleading (data.total || data.results?.length)

5. Association Type Hardcoding
In app/api/mcp/tools/hubspot.ts:736-748:

const associationTypeMap: Record<string, Record<string, number>> = {
  contacts: { companies: 1, deals: 3 },
  companies: { contacts: 2, deals: 5 },
  deals: { contacts: 4, companies: 6 },
};

These are HubSpot's default association types but may break if custom associations are needed. Consider:

  • Documenting this limitation
  • Adding a parameter for custom association types

6. Default Limits Inconsistency

  • Most endpoints default to limit: 20
  • hubspot_list_owners defaults to limit: 100
  • Consider standardizing or documenting the rationale

Low Priority

7. Property Selection
Default properties are hardcoded (e.g., lines 130-136). Consider:

  • Documenting available properties
  • Adding common properties as constants
  • Supporting wildcards or "all properties" option

8. Input Validation
The lifecyclestage enum in hubspot_create_contact (lines 222-231) is good, but:

  • Consider validating closedate format (ISO 8601)
  • Add validation for email format consistency
  • Validate dealstage and pipeline IDs exist

9. Logging Consistency

  • Some operations log success (e.g., line 258: "Contact created")
  • Others don't log (e.g., hubspot_get_contact)
  • Consider consistent logging for all mutations

🔐 Security Assessment

Pass - No critical security issues found:

  • Proper authentication via OAuth tokens
  • No SQL injection risks (using Drizzle ORM)
  • No XSS vulnerabilities (JSON APIs)
  • Proper organization isolation
  • Secrets managed via secretsService

Minor note: Ensure HubSpot API responses don't contain sensitive data that shouldn't be logged.

🧪 Testing

Good coverage of:

  • ✅ Tool registration (15 tools)
  • ✅ Connection status checks
  • ✅ CRUD operations
  • ✅ Error handling
  • ✅ Organization isolation

Missing:

  • ⚠️ Integration tests with actual HubSpot sandbox
  • ⚠️ Edge cases (empty results, malformed responses)
  • ⚠️ Concurrent request handling
  • ⚠️ Token refresh scenarios

📝 Code Style

Adheres to CLAUDE.md conventions:

  • ✅ Uses Bun runtime
  • ✅ Proper TypeScript typing
  • ✅ Drizzle ORM for data access
  • ✅ Consistent error handling
  • ✅ Proper module structure

Minor: Remove trailing commas that were inadvertently changed (e.g., app/api/mcp/route.ts:95 and 183).

🎯 Recommendation

Approve with minor revisions

The implementation is solid and follows best practices. Address the high-priority items before merging:

  1. Verify deal amount type consistency
  2. Consider rate limit handling
  3. Document association type limitations

The medium and low priority items can be follow-up improvements.

📊 Metrics

  • Files changed: 14
  • Lines added: 2,254
  • Lines removed: 39
  • Test coverage: Good (638 test lines)
  • Commits: 14 (clean history)

Great work on this integration! 🚀

@samarth30
Copy link
Member Author

Claude says: Addressed the critical and important items from the Claude CI review:

  1. Deal amount type mismatch (Critical Feat: Auth & Chat: Conversation History, Credit Tracking, Error Handling #1) — Fixed z.string() to z.number() in standalone route.ts to match hubspot.ts
  2. Inconsistent default limits (Critical feat: Add versioned /api/v1 endpoints with secure auth, credit tracking, and usage auditing #2) — Standardized all standalone endpoint defaults from 10 to 20
  3. Missing pagination in search ops (Important feat : Add generation metadata tracking and auditing across chat, image, and video APIs #3) — Added paging: data.paging to all 3 search operations in both implementations
  4. Error response parsing (Important feat: add credit packs, Stripe checkout , Credit tracking #4) — Added logger.warn fallback when error response JSON parsing fails

Items 5-11 (association type docs, rate limiting docs, JSDoc, test expansion, code dedup) are acknowledged as nice-to-haves and can be addressed in follow-up work.

Claude

@samarth30 samarth30 requested a review from 0xbbjoker February 12, 2026 14:57
@samarth30 samarth30 marked this pull request as ready for review February 16, 2026 13:51
@claude
Copy link

claude bot commented Feb 16, 2026

Code Review: HubSpot OAuth Integration (PR #283)

Thanks for this comprehensive addition of HubSpot OAuth integration! I've reviewed all 2,235 lines of changes across 14 files. Overall, the implementation follows the existing patterns well, but I've identified several critical issues that should be addressed before merging.

Critical Security Issues 🔴

1. Token Exposure in Error Responses

File: lib/services/oauth/providers/oauth2.ts:150-165

The fetchTokenInfo() function constructs URLs with access tokens that could be exposed in error logs:

const url = `${baseUrl.replace(/\/$/, "")}/${encodeURIComponent(accessToken)}`;

Risk: If the request fails, error messages may contain the token.

Fix: Redact tokens from error messages before logging or sanitize the URL.

2. Race Condition in OAuth Connection Storage

File: lib/services/oauth/providers/oauth2.ts:707-740

The storeConnection function checks for existing users OUTSIDE the transaction:

const existingByPlatformUser = await dbWrite.select(...).where(...);
// ... code that could take time ...
if (existingByPlatformUser.length > 0 && existingByPlatformUser[0].user_id !== userId) {
  throw new Error("OAUTH_ACCOUNT_ALREADY_LINKED");
}
// Later: transaction starts

Risk: Concurrent OAuth callbacks could bypass the duplicate check.

Fix: Move the duplicate check into a transaction using SELECT FOR UPDATE or use the onConflictDoUpdate check exclusively.

3. Secret Cleanup Before Transaction Commit

File: lib/services/oauth/providers/oauth2.ts:922-924

if (pendingRevocation) {
  await revokeConnectionsSecrets(pendingRevocation.connections, pendingRevocation.reason);
}
// Then transaction runs...

Risk: If the transaction fails after secrets are deleted, database records will reference non-existent secrets.

Fix: Move secret revocation to AFTER successful transaction commit, or include it in the same transaction.


High Priority Issues 🟡

4. No Rate Limiting on HubSpot API Calls

Files: app/api/mcp/tools/hubspot.ts, app/api/mcps/hubspot/[transport]/route.ts

HubSpot has strict rate limits (15 requests/10 seconds per token), but the tools make direct API calls without throttling.

Fix: Implement rate limiting middleware or add backoff/retry logic for 429 responses.

5. N+1 Query Pattern in OAuth Callback

File: lib/services/oauth/providers/oauth2.ts:708-762

Multiple sequential database queries for each OAuth callback:

  • Query for existing platform user
  • Query for user connections
  • Multiple insert/update operations

Fix: Combine queries or use a single transaction with CTEs.

6. Type Safety Issues with any

File: app/api/mcp/tools/hubspot.ts (lines 150, 341, 665+)

Extensive use of any types defeats TypeScript's safety:

contacts: data.results?.map((c: any) => ({
  id: c.id,
  ...c.properties,
}))

Fix: Define proper TypeScript interfaces for HubSpot API responses.


Code Quality & Testing

7. Incomplete Test Coverage

File: tests/unit/mcp-hubspot-tools.test.ts

Missing tests for:

  • hubspot_get_contact
  • hubspot_update_contact
  • hubspot_update_deal
  • All search operations (hubspot_search_contacts, hubspot_search_companies, hubspot_search_deals)
  • Pagination with after cursor
  • Edge cases (limit > 100, invalid contact IDs)

Current coverage: ~40% of tools tested
Expected: 80%+ coverage

8. Code Duplication Between Implementations

Files: app/api/mcp/tools/hubspot.ts vs app/api/mcps/hubspot/[transport]/route.ts

These files implement nearly identical functionality (765 vs 565 lines). Differences:

  • Different base path constants
  • Different error handling functions
  • Duplicate HubSpot API logic

Fix: Extract shared logic into a service layer.

9. Missing Input Validation

File: app/api/mcp/tools/hubspot.ts:244-250, 429-434

No validation on contact/company properties:

  • No length limits on strings
  • No sanitization of input (defense in depth)
  • No check for required fields beyond email

Fix: Add Zod schemas for property validation.


Performance & Reliability

10. No Backoff for API Rate Limits

When HubSpot returns 429 (Too Many Requests), the tools throw errors instead of retrying with exponential backoff.

Fix: Implement retry logic with exponential backoff for 429/503 responses.

11. Inefficient Secret Rotation

File: lib/services/oauth/providers/oauth2.ts:824-880

The secret rotation logic attempts to rotate, then creates new secrets if rotation fails, potentially creating orphaned secrets.

Fix: Use upsert pattern or defer cleanup to a background job.


Documentation & Clarity

12. Missing Documentation

The following are undocumented:

  • Why HubSpot uses tokenInfo instead of userInfo endpoint
  • Source of hardcoded association type IDs (lines 736-748 in hubspot.ts)
  • Rate limiting recommendations for users
  • Default property selections

13. Inconsistent Error Handling

Different error response formats between implementations:

  • hubspot.ts uses errorResponse()
  • route.ts uses errorResult()

Both produce similar output but create maintenance burden.


Migration & Database

14. Empty Migration File

File: db/migrations/0029_add_user_platform_oauth_constraint.sql

This file is empty (0 additions/deletions). Is this intentional or was the migration applied manually?


Minor Issues

  1. Hardcoded Association Type IDs (hubspot.ts:736-748) - No validation these IDs match current HubSpot API
  2. Missing CSRF Rate Limiting (oauth2.ts:203-207) - State validation has no rate limiting on invalid attempts
  3. Inconsistent URL Construction - Some files use constants, others use hardcoded URLs
  4. No Token Type Verification (oauth2.ts:1078-1106) - Doesn't verify correct token type after refresh

Testing Checklist

Before merging, please verify:

  • All HubSpot tools have unit tests
  • Integration tests for OAuth flow with HubSpot
  • Rate limiting behavior tested
  • Concurrent OAuth callbacks handled correctly
  • Error cases (expired tokens, invalid scopes) tested
  • Pagination tested with large result sets

Positive Aspects ✅

  • Good adherence to existing OAuth provider patterns
  • Comprehensive HubSpot CRM coverage (contacts, companies, deals, associations)
  • Proper use of Zod for input validation
  • Clean separation of MCP transport handlers
  • Good logging coverage
  • Follows project structure conventions

Recommendation

Approve with required changes. The implementation is solid but has critical security and reliability issues that must be addressed:

  1. Fix the race condition in OAuth connection storage (Critical)
  2. Fix secret cleanup transaction ordering (Critical)
  3. Add rate limiting for HubSpot API calls (High)
  4. Expand test coverage to 80%+ (High)
  5. Add proper TypeScript types (Medium)

Would you like help implementing any of these fixes?

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 6 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

actor: "user",
},
tokenContentType: "form", // Linear requires x-www-form-urlencoded
tokenContentType: "json",
Copy link

Choose a reason for hiding this comment

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

Linear token exchange content type breaks OAuth flow

High Severity

The Linear provider's tokenContentType was changed from "form" to "json", removing the comment that explicitly stated "Linear requires x-www-form-urlencoded". Linear's token endpoint (https://api.linear.app/oauth/token) requires application/x-www-form-urlencoded content type per their documentation. With "json", the exchangeCodeForTokens function sends application/json, which will cause the token exchange to fail, completely breaking Linear OAuth.

Fix in Cursor Fix in Web

revoke: "https://slack.com/api/auth.revoke",
},
// Bot scopes only - these must also be added in Slack app's OAuth & Permissions
defaultScopes: ["chat:write", "channels:read", "users:read"],
Copy link

Choose a reason for hiding this comment

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

Slack userInfoMapping incompatible with new endpoint

High Severity

The Slack userInfo endpoint was changed from auth.test to users.identity, but the userInfoMapping still uses auth.test field names (id: "user_id", displayName: "user"). The users.identity endpoint returns a nested response where user data is under user.id and user.name, not at the top level. This means user ID extraction will return undefined, breaking Slack OAuth user identification. The stale comment about "Bot tokens" also confirms the mapping wasn't updated.

Fix in Cursor Fix in Web

try {
const orgId = getOrgId();
const properties: Record<string, string> = { dealname };
if (amount) properties.amount = amount;
Copy link

Choose a reason for hiding this comment

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

Deal amount zero skipped and type mismatch

Medium Severity

In the standalone hubspot_create_deal, properties is typed as Record<string, string> but amount is z.number(), causing a type mismatch on assignment. More importantly, if (amount) is falsy when amount === 0, silently dropping a valid zero-amount deal. The main hubspot.ts correctly uses Record<string, string | number> and if (amount !== undefined).

Fix in Cursor Fix in Web

},
{ capabilities: { tools: {} } },
{ basePath: "/api/mcps/hubspot", maxDuration: 60 }
);
Copy link

Choose a reason for hiding this comment

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

Duplicate HubSpot tools across two files

Medium Severity

The HubSpot CRM tool logic is fully duplicated between hubspot.ts (15 tools via registerHubSpotTools) and the standalone route.ts (~12 tools). The two implementations already diverge — route.ts has the amount type bug, fewer tools (missing hubspot_update_deal, hubspot_associate), and different response shapes. Shared business logic (API calls, response mapping) could be extracted to avoid these inconsistencies.

Additional Locations (1)

Fix in Cursor Fix in Web


const googleServer = mcpSettings?.servers?.google as { url?: string; type?: string };
expect(googleServer.url).toContain("/api/mcps/google/mcp");
expect(googleServer.url).toContain("/api/mcps/google/streamable-http");
Copy link

Choose a reason for hiding this comment

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

Test expects URL path not matching server config

Medium Severity

The test assertion was changed to expect the Google MCP server URL to contain /api/mcps/google/streamable-http, but MCP_SERVER_CONFIGS in runtime-factory.ts still defines the URL as /api/mcps/google/mcp. The transformMcpSettings method only prepends the base URL to relative paths — it doesn't replace mcp with the transport type. This test will fail.

Fix in Cursor Fix in Web

if (domain) properties.domain = domain;
if (industry) properties.industry = industry;
if (numberofemployees) properties.numberofemployees = numberofemployees;
if (annualrevenue) properties.annualrevenue = annualrevenue;
Copy link

Choose a reason for hiding this comment

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

Falsy check drops zero for numeric company fields

Low Severity

if (numberofemployees) and if (annualrevenue) use falsy checks on numeric values, silently dropping 0. This is inconsistent within the same file, where hubspot_create_deal correctly uses if (amount !== undefined). A startup with 0 annual revenue would have the field silently omitted.

Fix in Cursor Fix in Web

@standujar standujar closed this Feb 24, 2026
@lalalune lalalune reopened this Mar 9, 2026
@lalalune lalalune merged commit 57bd46c into dev Mar 15, 2026
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.

4 participants