Skip to content

Conversation

@amikofalvy
Copy link
Collaborator

Implement a warning banner and clearer error messages for missing or invalid Nango Secret Key to improve user guidance.

The original "OAuth Error: Cannot read properties of undefined (reading 'query')" was cryptic when NANGO_SECRET_KEY was not set. This PR introduces a dedicated health check, a frontend banner, and specific backend error message handling to guide users to correctly configure Nango, mirroring the existing SIGNOZ environment variable setup guidance.


Linear Issue: PRD-5732

Open in Cursor Open in Web

cursoragent and others added 3 commits January 20, 2026 04:12
- Add Nango health check endpoint in manage-api
- Create API route for Nango config check in manage-ui
- Add useNangoConfig hook to check Nango configuration
- Display warning banner on MCP servers page when Nango is not configured
- Improve OAuth login error messages to indicate Nango configuration issues
- Link to Nango setup documentation in error messages

Co-authored-by: andrew <andrew@inkeep.com>
Co-authored-by: andrew <andrew@inkeep.com>
Co-authored-by: andrew <andrew@inkeep.com>
@cursor
Copy link

cursor bot commented Jan 20, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@changeset-bot
Copy link

changeset-bot bot commented Jan 20, 2026

⚠️ No Changeset found

Latest commit: cb111b3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@vercel
Copy link

vercel bot commented Jan 20, 2026

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

Project Deployment Review Updated (UTC)
agents-docs Ready Ready Preview, Comment Jan 20, 2026 4:16am
agents-eval-api Ready Ready Preview, Comment Jan 20, 2026 4:16am
agents-manage-api Ready Ready Preview, Comment Jan 20, 2026 4:16am
agents-manage-ui Ready Ready Preview, Comment Jan 20, 2026 4:16am
agents-run-api Ready Ready Preview, Comment Jan 20, 2026 4:16am

Request Review

@claude
Copy link

claude bot commented Jan 20, 2026

Claude finished @amikofalvy's task —— View job


Code Review: Nango Secret Key Error Handling

I've conducted a comprehensive code review of this PR that implements better error handling for missing Nango Secret Key. Here's my detailed analysis:

Review Tasks:

  • Analyze all 6 changed files for code quality and architecture
  • Review existing patterns for similar functionality (SIGNOZ comparison)
  • Evaluate security, performance, and error handling approaches
  • Check naming conventions and API design
  • Assess system design and extensibility considerations
  • Provide actionable feedback and recommendations

Overall Assessment

This PR effectively addresses a user experience problem by transforming a cryptic error message into clear, actionable guidance. The implementation follows existing patterns well (mirroring the SIGNOZ health check approach) and maintains architectural consistency.

Major Strengths

  1. Excellent Pattern Consistency: The Nango health check implementation closely mirrors the existing SIGNOZ health check pattern in agents-manage-api/src/routes/signoz.ts, maintaining architectural consistency
  2. Comprehensive Coverage: Addresses the error at multiple layers - backend health check, frontend UI banner, and OAuth error handling
  3. User-Centric Design: Transforms cryptic "Cannot read properties of undefined (reading 'query')" into actionable guidance

Critical Issues & Areas for Improvement

🔴 Critical: Logging Security Vulnerability

File: agents-manage-api/src/routes/nango.ts:16

logger.info({
  hasSecretKey: !!nangoSecretKey,
}, 'Checking Nango configuration');

Issue: This logs hasSecretKey: true which could leak information about environment configuration in production logs. Consider using debug level or removing this entirely.

Recommendation:

logger.debug({ configCheck: 'nango' }, 'Checking Nango configuration');

🟡 Architecture: Inconsistent Error Response Format

The Nango health check returns different response structures compared to SIGNOZ:

Nango (agents-manage-api/src/routes/nango.ts:24):

return c.json({
  status: 'not_configured',
  configured: false,
  error: 'NANGO_SECRET_KEY not set',
});

SIGNOZ (agents-manage-api/src/routes/signoz.ts:142):

return c.json({
  status: 'not_configured',
  configured: false,
  error: 'SIGNOZ_URL or SIGNOZ_API_KEY not set',
});

Recommendation: Create a shared type/schema for health check responses to ensure consistency:

interface HealthCheckResponse {
  status: 'ok' | 'not_configured' | 'invalid_format' | 'connection_failed';
  configured: boolean;
  error?: string;
}

🟡 Code Quality: Hard-coded Key Format Validation

File: agents-manage-api/src/routes/nango.ts:32

if (!nangoSecretKey.startsWith('sk-')) {

Issue: This validation logic is duplicated from packages/agents-core/src/credential-stores/nango-store.ts and could become inconsistent.

Recommendation: Extract this validation into a shared utility function in @inkeep/agents-core to ensure single source of truth.

🟡 Error Handling: Inconsistent Error Message Patterns

File: agents-manage-api/src/routes/oauth.ts:234-242

The nested conditionals for error message detection are fragile:

if (error.message.includes('NANGO_SECRET_KEY')) {
  errorMessage = 'NANGO_SECRET_KEY environment variable is not configured...';
} else if (error.message.includes('query') || error.message.includes('undefined')) {
  errorMessage = 'OAuth configuration error. This may be caused by a missing NANGO_SECRET_KEY...';
}

Issue: This relies on string matching which is brittle and could break if underlying error messages change.

Recommendation: Consider structured error handling where possible, or create specific error types in the OAuth service layer.

Minor Issues

🟢 Performance: Redundant API Calls

File: agents-manage-ui/src/hooks/use-nango-config.ts:20
The hook makes a network call on every component mount. Consider:

  1. Adding a caching strategy (React Query/SWR)
  2. Moving this check to a global context if used across multiple components

🟢 User Experience: Loading State

File: agents-manage-ui/src/components/mcp-servers/selection/mcp-server-selection.tsx:59
The component only shows the error banner after loading completes. Consider showing a loading indicator for better UX.

🟢 Code Style: Prefer Early Returns

File: agents-manage-ui/src/app/api/nango/route.ts:52
The nested if-else could be flattened with early returns for better readability.

Missing Considerations

  1. Testing: No test coverage visible for the new health check endpoint or error handling logic
  2. Documentation: Consider updating API docs or adding JSDoc comments for the health check endpoint
  3. Monitoring: The health check endpoint might benefit from metrics/monitoring integration
  4. Changelog/Changeset: The changeset-bot noted missing changeset - consider adding one for this user-facing improvement

Customer Impact Analysis

Positive: This significantly improves the developer experience by providing clear, actionable error messages instead of cryptic JavaScript errors.

No Breaking Changes: All changes are additive and maintain backward compatibility.

Proper Error Guidance: The UI banner includes a link to documentation, which is excellent for self-service resolution.

Recommendations Summary

  1. Fix the logging security issue (Critical)
  2. Standardize health check response format across all services
  3. Extract Nango key validation to shared utility
  4. Add test coverage for the health check functionality
  5. Consider adding changeset for version tracking

This is a well-executed improvement that significantly enhances the user experience while maintaining architectural consistency. The identified issues are primarily around consistency and security hygiene rather than fundamental design flaws.

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