Skip to content

fix circular deps#840

Merged
madster456 merged 3 commits intodevfrom
fix_api-overview
Aug 12, 2025
Merged

fix circular deps#840
madster456 merged 3 commits intodevfrom
fix_api-overview

Conversation

@madster456
Copy link
Copy Markdown
Collaborator

@madster456 madster456 commented Aug 12, 2025


Important

Fixes circular dependencies by restructuring OpenAPI type definitions and updating API paths, with enhancements to the API Explorer.

  • Breaking Changes:
    • MCP API endpoints are now prefixed with /api/internal instead of /api.
  • New Features:
    • API Explorer now supports building JSON request bodies from individual fields.
    • Generated curl/JavaScript/Python snippets reflect the new body builder.
  • Bug Fixes:
    • Improved URL handling in the API Explorer to prevent errors when server URLs are missing.
  • Refactor:
    • Centralized OpenAPI type definitions into openapi-types.ts for consistency and reuse.
    • Updated imports in enhanced-api-page.tsx and openapi-utils.ts to use the new openapi-types.ts.

This description was created by Ellipsis for bb27147. You can customize this summary. It will automatically update as commits are pushed.


Summary by CodeRabbit

  • Refactor

    • Centralized OpenAPI type definitions into a shared module for consistency.
    • Updated internal tool API routing under an internal namespace; no user-facing behavior changes.
    • Improved URL handling with safer fallbacks.
    • Switched request builder to field-based JSON bodies for clearer, more reliable payload construction.
  • Documentation

    • Regenerated code examples (cURL/JS/Python) to reflect safer URL handling and structured JSON bodies.
    • Aligned docs components with shared types for improved maintainability.
  • Chores

    • Adjusted internal imports and paths to match new module locations.

@vercel
Copy link
Copy Markdown

vercel bot commented Aug 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Project Deployment Preview Comments Updated (UTC)
stack-backend Ready Preview Comment Aug 12, 2025 5:24pm
stack-dashboard Ready Preview Comment Aug 12, 2025 5:24pm
stack-demo Ready Preview Comment Aug 12, 2025 5:24pm
stack-docs Ready Preview Comment Aug 12, 2025 5:24pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Aug 12, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

The PR centralizes OpenAPI types into a new shared module, updates imports accordingly, refactors request body handling to field-based JSON assembly, adds safer optional chaining for server URLs, and switches MCP browser/API paths to use /api/internal, including a basePath and import path adjustment in the internal route.

Changes

Cohort / File(s) Summary
MCP internal routing update
docs/src/app/mcp-browser/page.tsx, docs/src/app/api/internal/[transport]/route.ts
Switched MCP endpoint from /api/mcp to /api/internal/mcp; updated handler basePath to /api/internal and adjusted lib/source import path depth.
OpenAPI types centralization and usage
docs/src/lib/openapi-types.ts, docs/src/components/api/enhanced-api-page.tsx
Added shared OpenAPI type module exporting OpenAPISchema/Spec/Operation/Parameter. Replaced in-file type definitions with imports; refactored request body from string to bodyFields; updated code generators and URL resolution with optional chaining.
OpenAPI utils import alignment
docs/src/lib/openapi-utils.ts
Updated type import source from enhanced-api-page to the new openapi-types module; no functional changes.

Sequence Diagram(s)

sequenceDiagram
  participant UI as MCP Browser (page.tsx)
  participant API as Next.js Route /api/internal/[transport]
  participant MCP as MCP Handler

  UI->>API: POST /api/internal/mcp { tool, args }
  API->>MCP: Dispatch request with basePath=/api/internal
  MCP-->>API: Tool response
  API-->>UI: JSON result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

A nibble of types in a shared little den,
URLs hop inward, from outer to in.
Fields sprout a body, JSON in bloom,
Chains made optional sweep worries from the room.
Thump-thump commits—my paws approve,
Carrots for code that’s tidy and smooth. 🥕✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_api-overview

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Aug 12, 2025

Claude finished @madster456's task —— View job


Code Review for Circular Dependency Fix

Reviewing the PR changes to fix circular dependencies:

Todo List:

  • Read and analyze all changed files
  • Review the circular dependency fix approach
  • Check for potential bugs or issues
  • Evaluate code quality and best practices
  • Assess security and performance considerations
  • Provide comprehensive feedback

📝 Overall Assessment

This PR effectively addresses circular dependency issues through well-structured refactoring. The approach of extracting type definitions and restructuring imports is sound and follows best practices.

🔄 Refactoring Analysis

File Structure Changes:

  • route.ts relocated from /api/[transport]/ to /api/mcp/[transport]/ - logical separation
  • ✅ New openapi-types.ts - excellent extraction of shared types
  • ✅ Updated openapi-utils.ts - now uses the new type definitions
  • ✅ Path updates in page.tsx - correctly reflects new endpoint structure

Type Extraction Strategy:

  • ✅ All OpenAPI-related types moved to dedicated file
  • ✅ Clean separation of concerns
  • ✅ Eliminates circular dependencies in enhanced-api-page.tsx (reduced by 71 lines!)

Code Quality Highlights

  1. Clean Type Definitions (openapi-types.ts:1-75):

    • Comprehensive TypeScript types
    • Proper optional property handling
    • Good use of union types and generics
  2. Route Restructuring ([transport]/route.ts:249):

    • Correct basePath update to /api/mcp
    • Maintains all existing functionality
  3. URL Updates (page.tsx:38):

    • Properly updated fetch URL from /api/mcp to /api/mcp/mcp
    • Consistent with new route structure

🐛 Potential Issues Found

  1. Optional Chaining Enhancement (enhanced-api-page.tsx:208):

    const baseUrl = spec?.servers?.[0]?.url || '';

    Good: Properly handles potential undefined servers array

  2. Error Handling:

    • ✅ Maintained error handling in all affected files
    • ✅ No degradation in user experience

🚀 Performance Considerations

  • Bundle Size: Type extraction reduces duplicate code
  • Import Tree: Cleaner import structure should improve tree-shaking
  • Runtime Performance: No impact on runtime performance

🔒 Security Assessment

  • No Security Issues: The refactoring doesn't introduce security vulnerabilities
  • Input Validation: Maintained proper input validation in route handlers
  • CORS/Headers: No changes to security-related headers or CORS configuration

📋 Recommendations

  1. ✅ Approved: The circular dependency fix is well-implemented
  2. Testing: Ensure the MCP browser functionality works with the new endpoint path
  3. Documentation: Consider updating any API documentation that references the old paths

🎯 Test Coverage

Recommended Tests:

  • Verify /api/mcp/mcp endpoint responds correctly
  • Test MCP browser page loads and functions properly
  • Confirm OpenAPI type imports work as expected
  • Validate no circular dependency warnings in build

🏆 Summary

Excellent refactoring work! This PR successfully:

  • ✅ Eliminates circular dependencies
  • ✅ Improves code organization
  • ✅ Maintains all existing functionality
  • ✅ Follows TypeScript best practices
  • ✅ Has no performance regression

The type extraction pattern used here could serve as a template for similar refactoring efforts across the codebase.


Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR addresses circular dependency issues in the docs application by refactoring OpenAPI type definitions and reorganizing the MCP (Model Context Protocol) API routing structure.

The main changes include:

  1. Type Definition Extraction: Created a new openapi-types.ts file containing comprehensive TypeScript types for OpenAPI 3.x specifications (OpenAPISchema, OpenAPISpec, OpenAPIOperation, OpenAPIParameter). This allows both enhanced-api-page.tsx and openapi-utils.ts to import shared types without creating circular dependencies.

  2. Import Path Updates: Updated import statements in openapi-utils.ts and enhanced-api-page.tsx to reference the new centralized type definitions. Added defensive programming improvements with optional chaining for spec.servers?.[0]?.url access patterns.

  3. MCP Route Reorganization: Moved the MCP API handler from /api/[transport]/route.ts to /api/mcp/[transport]/route.ts, creating better separation between general API endpoints and MCP-specific functionality. Updated the basePath configuration from /api to /api/mcp and adjusted import paths accordingly.

  4. Client Endpoint Fix: Updated the MCP tool API call in mcp-browser/page.tsx from /api/mcp to /api/mcp/mcp to align with the new routing structure.

These changes follow standard TypeScript patterns for breaking circular dependencies by extracting shared types into separate modules, while the route reorganization creates cleaner separation of concerns in the API structure.

Confidence score: 4/5

  • This PR addresses architectural issues with a well-structured approach to breaking circular dependencies
  • The type extraction and route reorganization are implemented correctly with proper import path adjustments
  • Pay close attention to the MCP route handler file which uses relative file path operations that could be fragile in production environments

5 files reviewed, 3 comments

Edit Code Review Bot Settings | Greptile

@recurseml
Copy link
Copy Markdown

recurseml bot commented Aug 12, 2025

Review by RecurseML

✅ Files analyzed, no issues (5)

docs/src/lib/openapi-types.ts
docs/src/components/api/enhanced-api-page.tsx
docs/src/app/api/mcp/[transport]/route.ts
docs/src/app/mcp-browser/page.tsx
docs/src/lib/openapi-utils.ts

🗒️ View all ignored comments in this repo
  • The constraint 'TokenStoreType extends string' is too restrictive. It should likely be 'TokenStoreType extends string | object' to match the condition check in line 113 where TokenStoreType is checked against {}
  • Return type mismatch - the interface declares useUsers() returning ServerUser[] but the Team interface that this extends declares useUsers() returning TeamUser[]
  • There is a syntax error in the super constructor call due to the ellipsis operator used incorrectly. Objects aren't being merged correctly. This syntax usage can lead to runtime errors when trying to pass the merged object to 'super()'. Verify that the intended alterations to the object occur before or outside of the super() call if needed.
  • Throwing an error when no active span is found is too aggressive. The log function should gracefully fallback to console.log or another logging mechanism when there's no active span, since not all execution contexts will have an active span. This makes the code less resilient and could break functionality in non-traced environments.

📚 Relevant Docs

  • Function sets backendContext with a new configuration but doesn't pass 'defaultProjectKeys'. Since defaultProjectKeys is required in the type definition and cannot be updated (throws error if tried to set), this will cause a type error.
  • The schema is using array syntax for pick() which is incorrect for Yup schemas. The pick() method in Yup expects individual arguments, not an array. Should be changed to: emailConfigSchema.pick('type', 'host', 'port', 'username', 'sender_name', 'sender_email')

📚 Relevant Docs

  • Creating a refresh token with current timestamp as expiration means it expires immediately. Should set a future date for token expiration.
  • The 'tools' object is initialized as an empty object, even though 'tools' is presumably expected to contain tool definitions. This could cause the server capabilities to lack necessary tool configurations, thus potentially impacting functionalities that depend on certain tool setups.

📚 Relevant Docs

  • 'STACK_SECRET_SERVER_KEY' is potentially being included in every request header without checking its existence again here. Although it's checked during initialization, this could lead to security issues as it's exposed in all communications where the header is logged or captured.

📚 Relevant Docs

  • When adding 'use client' directive at the beginning, it doesn't check if file.text already contains the 'use client' directive. This could lead to duplicate 'use client' directives if the file already has one.

📚 Relevant Docs

Discord

Copy link
Copy Markdown
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 (1)
docs/src/components/api/enhanced-api-page.tsx (1)

208-209: Add null check before accessing servers array.

While the optional chaining (spec?.servers?.[0]?.url) prevents errors, consider providing a more meaningful fallback or validation when the servers array is empty or undefined. An empty base URL could lead to confusing behavior.

Consider adding explicit validation:

-      const baseUrl = spec?.servers?.[0]?.url || '';
+      const baseUrl = spec?.servers?.[0]?.url || '';
+      if (!baseUrl && spec?.servers?.length) {
+        console.warn('No valid server URL found in OpenAPI specification');
+      }

Also applies to: 388-389, 434-435, 484-485

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34a4658 and fde852b.

📒 Files selected for processing (5)
  • docs/src/app/api/mcp/[transport]/route.ts (2 hunks)
  • docs/src/app/mcp-browser/page.tsx (1 hunks)
  • docs/src/components/api/enhanced-api-page.tsx (5 hunks)
  • docs/src/lib/openapi-types.ts (1 hunks)
  • docs/src/lib/openapi-utils.ts (1 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). (9)
  • GitHub Check: build (22.x)
  • GitHub Check: setup-tests
  • GitHub Check: docker
  • GitHub Check: docker
  • GitHub Check: restart-dev-and-test
  • GitHub Check: lint_and_build (latest)
  • GitHub Check: all-good
  • GitHub Check: build (22.x)
  • GitHub Check: Security Check
🔇 Additional comments (7)
docs/src/lib/openapi-types.ts (1)

1-75: Good extraction of OpenAPI types for better modularity!

Extracting these types into a dedicated module eliminates circular dependencies and improves code organization. This is a positive architectural change that follows the single responsibility principle.

docs/src/lib/openapi-utils.ts (1)

1-1: Import path correctly updated to resolve circular dependency.

The import now properly references the new openapi-types module from the same directory, which breaks the previous circular dependency chain between openapi-utils.ts and enhanced-api-page.tsx.

docs/src/components/api/enhanced-api-page.tsx (2)

5-5: Import correctly updated to use the extracted types module.

The import path now references the centralized types from ../../lib/openapi-types, completing the circular dependency fix.


22-22: Good refactoring of request body handling to use individual fields.

Changing from a single body string to bodyFields: Record<string, unknown> provides better type safety and more granular control over request body construction. This makes the API testing interface more user-friendly by allowing field-by-field editing.

docs/src/app/mcp-browser/page.tsx (1)

38-38: API endpoint path correctly updated to match new routing structure.

The MCP tool API endpoint has been updated from /api/mcp to /api/mcp/mcp to align with the new basePath configuration in the route handler.

docs/src/app/api/mcp/[transport]/route.ts (2)

249-249: Base path correctly updated to align with new routing structure.

The basePath change from /api to /api/mcp properly configures the MCP handler to match the new endpoint structure, ensuring the route correctly handles requests to /api/mcp/[transport].


4-4: Import path correctly resolves to docs/lib/source.ts

The import "../../../../../lib/source" from docs/src/app/api/mcp/[transport]/route.ts climbs five levels to the docs directory—where docs/lib/source.ts exists—so no change is needed.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Aug 12, 2025

Claude finished @madster456's task —— View job


Code Review for Circular Dependency Fix

Todo List:

  • Read and analyze all changed files
  • Review the circular dependency fix approach
  • Check for potential bugs or issues
  • Evaluate code quality and best practices
  • Assess security and performance considerations
  • Provide comprehensive feedback

🔄 Refactoring Analysis

File Structure Changes:

  • route.ts relocated from /api/[transport]/ to /api/internal/[transport]/ - logical separation
  • ✅ New openapi-types.ts - excellent extraction of shared types
  • ✅ Updated openapi-utils.ts - now uses the new type definitions
  • ✅ Path updates in page.tsx - correctly reflects new endpoint structure

Type Extraction Strategy:

  • ✅ All OpenAPI-related types moved to dedicated file
  • ✅ Clean separation of concerns
  • ✅ Eliminates circular dependencies in enhanced-api-page.tsx (reduced by 71 lines!)

Code Quality Highlights

  1. Clean Type Definitions (openapi-types.ts:1-75):

    • Comprehensive TypeScript types
    • Proper optional property handling
    • Good use of union types and generics
  2. Route Restructuring ([transport]/route.ts:249):

    • Correct basePath update to /api/internal
    • Maintains all existing functionality
  3. URL Updates (page.tsx:38):

    • Properly updated fetch URL from /api/mcp to /api/internal/mcp
    • Consistent with new route structure

🐛 Potential Issues Found

  1. Optional Chaining Enhancement (enhanced-api-page.tsx:208):

    const baseUrl = spec?.servers?.[0]?.url || '';

    Good: Properly handles potential undefined servers array

  2. Error Handling:

    • ✅ Maintained error handling in all affected files
    • ✅ No degradation in user experience

🚀 Performance Considerations

  • Bundle Size: Type extraction reduces duplicate code
  • Import Tree: Cleaner import structure should improve tree-shaking
  • Runtime Performance: No impact on runtime performance

🔒 Security Assessment

  • No Security Issues: The refactoring doesn't introduce security vulnerabilities
  • Input Validation: Maintained proper input validation in route handlers
  • CORS/Headers: No changes to security-related headers or CORS configuration

📋 Recommendations

  1. ✅ Approved: The circular dependency fix is well-implemented
  2. Testing: Ensure the MCP browser functionality works with the new endpoint path
  3. Documentation: Consider updating any API documentation that references the old paths

🎯 Test Coverage

Recommended Tests:

  • Verify /api/internal/mcp endpoint responds correctly
  • Test MCP browser page loads and functions properly
  • Confirm OpenAPI type imports work as expected
  • Validate no circular dependency warnings in build

🏆 Summary

Excellent refactoring work! This PR successfully:

  • ✅ Eliminates circular dependencies
  • ✅ Improves code organization
  • ✅ Maintains all existing functionality
  • ✅ Follows TypeScript best practices
  • ✅ Has no performance regression

The type extraction pattern used here could serve as a template for similar refactoring efforts across the codebase.


Copy link
Copy Markdown
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: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fde852b and bb27147.

📒 Files selected for processing (2)
  • docs/src/app/api/internal/[transport]/route.ts (2 hunks)
  • docs/src/app/mcp-browser/page.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/src/app/mcp-browser/page.tsx
⏰ 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). (10)
  • GitHub Check: Security Check
  • GitHub Check: restart-dev-and-test
  • GitHub Check: claude-review
  • GitHub Check: all-good
  • GitHub Check: docker
  • GitHub Check: build (22.x)
  • GitHub Check: lint_and_build (latest)
  • GitHub Check: build (22.x)
  • GitHub Check: setup-tests
  • GitHub Check: docker
🔇 Additional comments (1)
docs/src/app/api/internal/[transport]/route.ts (1)

249-252: Base path update to /api/internal matches route location — LGTM

This aligns the handler with docs/src/app/api/internal/[transport]/route.ts and the internal MCP client path. Good change.

@madster456 madster456 merged commit 0580a56 into dev Aug 12, 2025
21 checks passed
@madster456 madster456 deleted the fix_api-overview branch August 12, 2025 18:00
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