Repository navigation
refactor(types): consolidate type system — resolve collisions, break circular deps, absorb server types - #921
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughLarge-scale TypeScript type-system refactor: client-facing types were renamed with a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
Contributor
Documentation Validation Results🚀 Documentation validation passed!
📦 Build artifact uploaded successfully. Ready for deployment preview. Commit: |
There was a problem hiding this comment.
Pull request overview
Consolidates and deconflicts NeuroLink’s TypeScript type system by centralizing canonical type definitions, breaking type-level circular dependencies, and adding compatibility shims to keep existing imports working.
Changes:
- Deduplicates several shared aliases by moving canonical definitions to
types/common.tsand updating consumers. - Renames colliding client-facing types with a
Client*prefix and updates client modules accordingly. - Introduces canonical server adapter types in
types/serverTypes.tsand convertsserver/types.tsinto a compatibility shim.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/types/typeAliases.ts | Removes duplicate aliases and imports canonical types from common.ts. |
| src/lib/types/streamTypes.ts | Adjusts imports to reduce circular deps (notably EvaluationData). |
| src/lib/types/serverTypes.ts | Adds canonical server adapter type definitions (new file). |
| src/lib/types/sdkTypes.ts | Updates stream type export aliases to align with renamed types. |
| src/lib/types/ragTypes.ts | Adds RAGConfig in canonical types and re-exports needed RAG types. |
| src/lib/types/index.ts | Updates barrel exports to reflect renamed/centralized types; exports server types. |
| src/lib/types/configTypes.ts | Switches MCPToolRegistry import to import type to avoid runtime dependency. |
| src/lib/types/clientTypes.ts | Renames colliding client types to Client* variants and updates internal references. |
| src/lib/types/cli.ts | Updates EvaluationData import to avoid circular import through ../index.js. |
| src/lib/types/analytics.ts | Renames ErrorInfo → AnalyticsErrorInfo to avoid collision with common.ts. |
| src/lib/server/types.ts | Converts to shim via export * from canonical server types (but retains additional exports). |
| src/lib/rag/types.ts | Re-exports RAGConfig from canonical types/ragTypes.ts. |
| src/lib/client/wsClient.ts | Updates imports to use ClientStreamEvent / ClientStreamResult aliases. |
| src/lib/client/streamingClient.ts | Updates imports to use ClientStreamEvent / ClientStreamResult aliases. |
| src/lib/client/sseClient.ts | Updates imports to use ClientStreamEvent / ClientStreamResult aliases. |
| src/lib/client/reactHooks.tsx | Updates imports to use ClientChatMessage / ClientStreamEvent aliases. |
| src/lib/client/interceptors.ts | Updates middleware/retry types to Client* names. |
| src/lib/client/index.ts | Keeps external client API stable by re-aliasing Client* types back to legacy names. |
| src/lib/client/httpClient.ts | Updates internal typings to Client* names for responses/streaming/middleware. |
| src/lib/client/auth.ts | Updates OAuth/token typings to ClientOAuth2Config / ClientTokenRefreshResult. |
| docs/superpowers/plans/2026-04-03-type-system-consolidation.md | Adds an implementation plan document for the type-system consolidation effort. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import type { AIProviderName } from "../constants/enums.js"; | ||
| import type { EvaluationData } from "../index.js"; | ||
| import type { EvaluationData } from "./evaluation.js"; | ||
| import type { RAGConfig } from "../rag/types.js"; |
There was a problem hiding this comment.
streamTypes.ts still imports RAGConfig from ../rag/types.js. Since RAGConfig has been moved into the canonical types/ragTypes.ts, this import should come from ./ragTypes.js to avoid types/ depending on implementation modules (and to prevent reintroducing circular type dependencies).
Suggested change
| import type { RAGConfig } from "../rag/types.js"; | |
| import type { RAGConfig } from "./ragTypes.js"; |
Comment on lines
+6
to
+10
| // Re-export all types from canonical location | ||
| export * from "../types/serverTypes.js"; | ||
|
|
||
| // ============================================ | ||
| // Configuration Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Server adapter configuration | ||
| */ | ||
| export type ServerAdapterConfig = { | ||
| /** Server port (default: 3000) */ | ||
| port?: number; | ||
|
|
||
| /** Server host (default: "0.0.0.0") */ | ||
| host?: string; | ||
|
|
||
| /** Base path for all routes (default: "/api") */ | ||
| basePath?: string; | ||
|
|
||
| /** CORS configuration */ | ||
| cors?: CORSConfig; | ||
|
|
||
| /** Rate limiting configuration */ | ||
| rateLimit?: RateLimitConfig; | ||
|
|
||
| /** Body parser configuration */ | ||
| bodyParser?: BodyParserConfig; | ||
|
|
||
| /** Logging configuration */ | ||
| logging?: LoggingConfig; | ||
|
|
||
| /** Request timeout in milliseconds (default: 30000) */ | ||
| timeout?: number; | ||
|
|
||
| /** Enable metrics endpoint (default: true) */ | ||
| enableMetrics?: boolean; | ||
|
|
||
| /** Enable Swagger/OpenAPI documentation (default: false) */ | ||
| enableSwagger?: boolean; | ||
|
|
||
| /** Disable built-in health routes (use when registering healthRoutes separately) */ | ||
| disableBuiltInHealth?: boolean; | ||
|
|
||
| /** Stream redaction configuration (disabled by default) */ | ||
| redaction?: RedactionConfig; | ||
|
|
||
| /** Shutdown configuration for graceful shutdown behavior */ | ||
| shutdown?: ShutdownConfig; | ||
| }; | ||
|
|
||
| /** | ||
| * Required server adapter configuration (with defaults applied) | ||
| */ | ||
| export type RequiredServerAdapterConfig = { | ||
| port: number; | ||
| host: string; | ||
| basePath: string; | ||
| cors: RequiredCORSConfig; | ||
| rateLimit: RequiredRateLimitConfig; | ||
| bodyParser: RequiredBodyParserConfig; | ||
| logging: RequiredLoggingConfig; | ||
| timeout: number; | ||
| enableMetrics: boolean; | ||
| enableSwagger: boolean; | ||
| disableBuiltInHealth: boolean; | ||
| shutdown: RequiredShutdownConfig; | ||
| }; | ||
|
|
||
| /** | ||
| * CORS configuration | ||
| */ | ||
| export type CORSConfig = { | ||
| /** Enable CORS (default: true) */ | ||
| enabled?: boolean; | ||
|
|
||
| /** Allowed origins (default: ["*"]) */ | ||
| origins?: string[]; | ||
|
|
||
| /** Allowed HTTP methods */ | ||
| methods?: string[]; | ||
|
|
||
| /** Allowed headers */ | ||
| headers?: string[]; | ||
|
|
||
| /** Allow credentials */ | ||
| credentials?: boolean; | ||
|
|
||
| /** Preflight cache max age in seconds */ | ||
| maxAge?: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Required CORS configuration | ||
| */ | ||
| export type RequiredCORSConfig = { | ||
| enabled: boolean; | ||
| origins: string[]; | ||
| methods: string[]; | ||
| headers: string[]; | ||
| credentials: boolean; | ||
| maxAge: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Rate limiting configuration | ||
| */ | ||
| export type RateLimitConfig = { | ||
| /** Enable rate limiting (default: true) */ | ||
| enabled?: boolean; | ||
|
|
||
| /** Time window in milliseconds (default: 15 minutes) */ | ||
| windowMs?: number; | ||
|
|
||
| /** Maximum requests per window (default: 100) */ | ||
| maxRequests?: number; | ||
|
|
||
| /** Custom error message */ | ||
| message?: string; | ||
|
|
||
| /** Skip rate limiting for certain paths */ | ||
| skipPaths?: string[]; | ||
|
|
||
| /** Custom key generator function */ | ||
| keyGenerator?: (ctx: ServerContext) => string; | ||
| }; | ||
|
|
||
| /** | ||
| * Required rate limit configuration | ||
| */ | ||
| export type RequiredRateLimitConfig = { | ||
| enabled: boolean; | ||
| windowMs: number; | ||
| maxRequests: number; | ||
| message: string; | ||
| skipPaths?: string[]; | ||
| keyGenerator?: (ctx: ServerContext) => string; | ||
| }; | ||
|
|
||
| /** | ||
| * Body parser configuration | ||
| */ | ||
| export type BodyParserConfig = { | ||
| /** Enable body parsing (default: true) */ | ||
| enabled?: boolean; | ||
|
|
||
| /** Maximum body size (default: "10mb") */ | ||
| maxSize?: string; | ||
|
|
||
| /** JSON body limit (default: "10mb") */ | ||
| jsonLimit?: string; | ||
|
|
||
| /** Enable URL-encoded body parsing */ | ||
| urlEncoded?: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Required body parser configuration | ||
| */ | ||
| export type RequiredBodyParserConfig = { | ||
| enabled: boolean; | ||
| maxSize: string; | ||
| jsonLimit: string; | ||
| urlEncoded: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Logging configuration | ||
| */ | ||
| export type LoggingConfig = { | ||
| /** Enable request logging (default: true) */ | ||
| enabled?: boolean; | ||
|
|
||
| /** Log level */ | ||
| level?: "debug" | "info" | "warn" | "error"; | ||
|
|
||
| /** Include request body in logs */ | ||
| includeBody?: boolean; | ||
|
|
||
| /** Include response body in logs */ | ||
| includeResponse?: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Required logging configuration | ||
| */ | ||
| export type RequiredLoggingConfig = { | ||
| enabled: boolean; | ||
| level: "debug" | "info" | "warn" | "error"; | ||
| includeBody: boolean; | ||
| includeResponse: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Configuration for stream redaction | ||
| * | ||
| * IMPORTANT: Redaction is DISABLED by default (enabled: false) | ||
| * This is an opt-in security feature to prevent accidental data exposure. | ||
| */ | ||
| export type RedactionConfig = { | ||
| /** | ||
| * Enable stream redaction (default: false) | ||
| * | ||
| * When false, redactStreamChunk() returns chunks unchanged. | ||
| * Must be explicitly set to true to enable redaction. | ||
| */ | ||
| enabled?: boolean; | ||
|
|
||
| /** Additional field names to redact (case-insensitive) */ | ||
| additionalFields?: string[]; | ||
|
|
||
| /** Field names to preserve (not redact) */ | ||
| preserveFields?: string[]; | ||
|
|
||
| /** Whether to redact tool arguments when enabled (default: true) */ | ||
| redactToolArgs?: boolean; | ||
|
|
||
| /** Whether to redact tool results when enabled (default: true) */ | ||
| redactToolResults?: boolean; | ||
|
|
||
| /** Custom redaction placeholder (default: "[REDACTED]") */ | ||
| placeholder?: string; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // Request/Response Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Server request context | ||
| * Passed to all route handlers and middleware | ||
| */ | ||
| export type ServerContext = { | ||
| /** Unique request ID */ | ||
| requestId: string; | ||
|
|
||
| /** HTTP method */ | ||
| method: string; | ||
|
|
||
| /** Request path */ | ||
| path: string; | ||
|
|
||
| /** Request headers */ | ||
| headers: Record<string, string>; | ||
|
|
||
| /** Query parameters */ | ||
| query: Record<string, string>; | ||
|
|
||
| /** Path parameters */ | ||
| params: Record<string, string>; | ||
|
|
||
| /** Request body (parsed) */ | ||
| body?: unknown; | ||
|
|
||
| /** NeuroLink SDK instance */ | ||
| neurolink: NeuroLink; | ||
|
|
||
| /** Tool registry instance */ | ||
| toolRegistry: MCPToolRegistry; | ||
|
|
||
| /** External server manager (optional) */ | ||
| externalServerManager?: ExternalServerManager; | ||
|
|
||
| /** Request timestamp */ | ||
| timestamp: number; | ||
|
|
||
| /** Additional metadata */ | ||
| metadata: Record<string, unknown>; | ||
|
|
||
| /** User information (if authenticated) */ | ||
| user?: { | ||
| id: string; | ||
| email?: string; | ||
| roles?: string[]; | ||
| }; | ||
|
|
||
| /** Session information */ | ||
| session?: { | ||
| id: string; | ||
| data?: Record<string, JsonValue>; | ||
| }; | ||
|
|
||
| /** Abort signal for cancellation (set by abort signal middleware) */ | ||
| abortSignal?: AbortSignal; | ||
|
|
||
| /** Abort controller for manual cancellation (set by abort signal middleware) */ | ||
| abortController?: AbortController; | ||
|
|
||
| /** Raw framework response object (for framework-specific operations) */ | ||
| rawResponse?: unknown; | ||
|
|
||
| /** Raw framework request object (for framework-specific operations) */ | ||
| rawRequest?: unknown; | ||
|
|
||
| /** Response headers to be set (used by middleware to add headers) */ | ||
| responseHeaders?: Record<string, string>; | ||
|
|
||
| /** Redaction configuration (for stream redaction support) */ | ||
| redaction?: RedactionConfig; | ||
| }; | ||
|
|
||
| /** | ||
| * Server response object | ||
| */ | ||
| export type ServerResponse<T = unknown> = { | ||
| /** Response data */ | ||
| data?: T; | ||
|
|
||
| /** Error information */ | ||
| error?: { | ||
| code: string; | ||
| message: string; | ||
| details?: Record<string, unknown>; | ||
| }; | ||
|
|
||
| /** Response metadata */ | ||
| metadata?: { | ||
| requestId: string; | ||
| timestamp: string; | ||
| duration?: number; | ||
| }; | ||
| }; | ||
|
|
||
| /** | ||
| * Streaming response configuration | ||
| */ | ||
| export type StreamingConfig = { | ||
| /** Enable streaming response */ | ||
| enabled: boolean; | ||
|
|
||
| /** Content type for streaming */ | ||
| contentType?: "text/event-stream" | "application/x-ndjson"; | ||
|
|
||
| /** Keep-alive interval in milliseconds */ | ||
| keepAliveInterval?: number; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // Route Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * HTTP methods supported by server adapters | ||
| */ | ||
| export type HttpMethod = | ||
| | "GET" | ||
| | "POST" | ||
| | "PUT" | ||
| | "DELETE" | ||
| | "PATCH" | ||
| | "OPTIONS"; | ||
|
|
||
| /** | ||
| * Route deprecation information | ||
| */ | ||
| export type RouteDeprecation = { | ||
| /** Whether the route is deprecated */ | ||
| enabled: boolean; | ||
|
|
||
| /** Version when deprecated */ | ||
| since?: string; | ||
|
|
||
| /** Version when route will be removed */ | ||
| removeIn?: string; | ||
|
|
||
| /** Alternative route to use */ | ||
| alternative?: string; | ||
|
|
||
| /** Deprecation message */ | ||
| message?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Route definition | ||
| */ | ||
| export type RouteDefinition = { | ||
| /** HTTP method */ | ||
| method: HttpMethod; | ||
|
|
||
| /** Route path (supports parameters like :id) */ | ||
| path: string; | ||
|
|
||
| /** Route handler function */ | ||
| handler: RouteHandler; | ||
|
|
||
| /** Route description (for documentation) */ | ||
| description?: string; | ||
|
|
||
| /** Request schema (for validation) */ | ||
| requestSchema?: JsonObject; | ||
|
|
||
| /** Response schema (for documentation) */ | ||
| responseSchema?: JsonObject; | ||
|
|
||
| /** Authentication required */ | ||
| auth?: boolean; | ||
|
|
||
| /** Required roles */ | ||
| roles?: string[]; | ||
|
|
||
| /** Rate limit override for this route */ | ||
| rateLimit?: RateLimitConfig; | ||
|
|
||
| /** Streaming configuration */ | ||
| streaming?: StreamingConfig; | ||
|
|
||
| /** Route tags (for documentation) */ | ||
| tags?: string[]; | ||
|
|
||
| /** Route deprecation information */ | ||
| deprecated?: RouteDeprecation; | ||
| }; | ||
|
|
||
| /** | ||
| * Route handler function | ||
| */ | ||
| export type RouteHandler<T = unknown> = ( | ||
| ctx: ServerContext, | ||
| ) => Promise<T | ServerResponse<T> | AsyncIterable<unknown>>; | ||
|
|
||
| /** | ||
| * Route group for organizing related routes | ||
| */ | ||
| export type RouteGroup = { | ||
| /** Group prefix */ | ||
| prefix: string; | ||
|
|
||
| /** Routes in this group */ | ||
| routes: RouteDefinition[]; | ||
|
|
||
| /** Middleware specific to this group */ | ||
| middleware?: MiddlewareDefinition[]; | ||
|
|
||
| /** Group-level authentication */ | ||
| auth?: boolean; | ||
|
|
||
| /** Group-level roles */ | ||
| roles?: string[]; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // Middleware Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Middleware definition | ||
| */ | ||
| export type MiddlewareDefinition = { | ||
| /** Middleware name */ | ||
| name: string; | ||
|
|
||
| /** Execution order (lower = earlier) */ | ||
| order?: number; | ||
|
|
||
| /** Middleware handler */ | ||
| handler: MiddlewareHandler; | ||
|
|
||
| /** Paths to apply middleware to (default: all) */ | ||
| paths?: string[]; | ||
|
|
||
| /** Paths to exclude from middleware */ | ||
| excludePaths?: string[]; | ||
| }; | ||
|
|
||
| /** | ||
| * Middleware handler function | ||
| */ | ||
| export type MiddlewareHandler = ( | ||
| ctx: ServerContext, | ||
| next: () => Promise<unknown>, | ||
| ) => Promise<unknown>; | ||
|
|
||
| // ============================================ | ||
| // Event Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Server adapter events | ||
| */ | ||
| export type ServerAdapterEvents = { | ||
| /** Server initialized */ | ||
| initialized: { | ||
| config: ServerAdapterConfig; | ||
| routeCount: number; | ||
| middlewareCount: number; | ||
| }; | ||
|
|
||
| /** Server started */ | ||
| started: { | ||
| port: number; | ||
| host: string; | ||
| timestamp: Date; | ||
| }; | ||
|
|
||
| /** Server stopped */ | ||
| stopped: { | ||
| uptime: number; | ||
| timestamp: Date; | ||
| }; | ||
|
|
||
| /** Request received */ | ||
| request: { | ||
| requestId: string; | ||
| method: string; | ||
| path: string; | ||
| timestamp: Date; | ||
| }; | ||
|
|
||
| /** Response sent */ | ||
| response: { | ||
| requestId: string; | ||
| statusCode: number; | ||
| duration: number; | ||
| timestamp: Date; | ||
| }; | ||
|
|
||
| /** Error occurred */ | ||
| error: { | ||
| requestId?: string; | ||
| error: Error; | ||
| timestamp: Date; | ||
| }; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // API Request/Response Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Agent execution request | ||
| */ | ||
| export type AgentExecuteRequest = { | ||
| /** Input prompt or message */ | ||
| input: string | { text: string; images?: string[]; files?: string[] }; | ||
|
|
||
| /** Provider to use (optional) */ | ||
| provider?: string; | ||
|
|
||
| /** Model to use (optional) */ | ||
| model?: string; | ||
|
|
||
| /** System prompt (optional) */ | ||
| systemPrompt?: string; | ||
|
|
||
| /** Temperature (0-1) */ | ||
| temperature?: number; | ||
|
|
||
| /** Maximum tokens */ | ||
| maxTokens?: number; | ||
|
|
||
| /** Tools to enable */ | ||
| tools?: string[]; | ||
|
|
||
| /** Enable streaming */ | ||
| stream?: boolean; | ||
|
|
||
| /** Session ID for conversation memory */ | ||
| sessionId?: string; | ||
|
|
||
| /** User ID for context */ | ||
| userId?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Agent execution response | ||
| */ | ||
| export type AgentExecuteResponse = { | ||
| /** Generated content */ | ||
| content: string; | ||
|
|
||
| /** Provider used */ | ||
| provider: string; | ||
|
|
||
| /** Model used */ | ||
| model: string; | ||
|
|
||
| /** Token usage */ | ||
| usage?: { | ||
| /** Input tokens (also known as prompt tokens) */ | ||
| input: number; | ||
| /** Output tokens (also known as completion tokens) */ | ||
| output: number; | ||
| /** Total tokens used */ | ||
| total: number; | ||
| /** Cache creation tokens (if applicable) */ | ||
| cacheCreationTokens?: number; | ||
| /** Cache read tokens (if applicable) */ | ||
| cacheReadTokens?: number; | ||
| /** Reasoning tokens (if applicable) */ | ||
| reasoning?: number; | ||
| /** Cache savings percentage */ | ||
| cacheSavingsPercent?: number; | ||
| }; | ||
|
|
||
| /** Tool calls made */ | ||
| toolCalls?: Array<{ | ||
| name: string; | ||
| arguments: Record<string, unknown>; | ||
| result?: unknown; | ||
| }>; | ||
|
|
||
| /** Finish reason */ | ||
| finishReason?: string; | ||
|
|
||
| /** Response metadata */ | ||
| metadata?: Record<string, JsonValue>; | ||
| }; | ||
|
|
||
| /** | ||
| * Embed request (single text) | ||
| */ | ||
| export type EmbedRequest = { | ||
| /** Text to embed */ | ||
| text: string; | ||
|
|
||
| /** Provider to use (optional) */ | ||
| provider?: string; | ||
|
|
||
| /** Embedding model to use (optional) */ | ||
| model?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Embed response (single text) | ||
| */ | ||
| export type EmbedResponse = { | ||
| /** The embedding vector */ | ||
| embedding: number[]; | ||
|
|
||
| /** Provider used */ | ||
| provider: string; | ||
|
|
||
| /** Model used */ | ||
| model: string; | ||
|
|
||
| /** Embedding dimension */ | ||
| dimension: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Embed many request (batch texts) | ||
| */ | ||
| export type EmbedManyRequest = { | ||
| /** Texts to embed */ | ||
| texts: string[]; | ||
|
|
||
| /** Provider to use (optional) */ | ||
| provider?: string; | ||
|
|
||
| /** Embedding model to use (optional) */ | ||
| model?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Embed many response (batch texts) | ||
| */ | ||
| export type EmbedManyResponse = { | ||
| /** The embedding vectors */ | ||
| embeddings: number[][]; | ||
|
|
||
| /** Provider used */ | ||
| provider: string; | ||
|
|
||
| /** Model used */ | ||
| model: string; | ||
|
|
||
| /** Number of embeddings */ | ||
| count: number; | ||
|
|
||
| /** Embedding dimension */ | ||
| dimension: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Tool execution request | ||
| */ | ||
| export type ToolExecuteRequest = { | ||
| /** Tool name */ | ||
| name: string; | ||
|
|
||
| /** Tool arguments */ | ||
| arguments: Record<string, unknown>; | ||
|
|
||
| /** Session context */ | ||
| sessionId?: string; | ||
|
|
||
| /** User context */ | ||
| userId?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Tool execution response | ||
| */ | ||
| export type ToolExecuteResponse = { | ||
| /** Whether execution was successful */ | ||
| success: boolean; | ||
|
|
||
| /** Result data */ | ||
| data?: unknown; | ||
|
|
||
| /** Error message if failed */ | ||
| error?: string; | ||
|
|
||
| /** Execution duration in ms */ | ||
| duration: number; | ||
|
|
||
| /** Tool metadata */ | ||
| metadata?: Record<string, JsonValue>; | ||
| }; | ||
|
|
||
| /** | ||
| * MCP server status response | ||
| */ | ||
| export type MCPServerStatusResponse = { | ||
| /** Server ID */ | ||
| serverId: string; | ||
|
|
||
| /** Server name */ | ||
| name: string; | ||
|
|
||
| /** Connection status */ | ||
| status: ExternalMCPServerStatus; | ||
|
|
||
| /** Available tools count */ | ||
| toolCount: number; | ||
|
|
||
| /** Last health check time */ | ||
| lastHealthCheck?: string; | ||
|
|
||
| /** Error message if failed */ | ||
| error?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Health check response | ||
| */ | ||
| export type HealthResponse = { | ||
| /** Health status */ | ||
| status: "ok" | "degraded" | "unhealthy"; | ||
|
|
||
| /** Timestamp */ | ||
| timestamp: string; | ||
|
|
||
| /** Server uptime in milliseconds */ | ||
| uptime: number; | ||
|
|
||
| /** Version information */ | ||
| version: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Ready check response | ||
| */ | ||
| export type ReadyResponse = { | ||
| /** Ready status */ | ||
| ready: boolean; | ||
|
|
||
| /** Timestamp */ | ||
| timestamp: string; | ||
|
|
||
| /** Service status */ | ||
| services: { | ||
| neurolink: boolean; | ||
| tools: boolean; | ||
| externalServers: boolean; | ||
| }; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // Factory Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Supported server frameworks | ||
| */ | ||
| export type ServerFramework = "hono" | "express" | "fastify" | "koa"; | ||
|
|
||
| /** | ||
| * Server adapter factory options | ||
| */ | ||
| export type ServerAdapterFactoryOptions = { | ||
| /** Framework to use */ | ||
| framework: ServerFramework; | ||
|
|
||
| /** NeuroLink instance */ | ||
| neurolink: NeuroLink; | ||
|
|
||
| /** Server configuration */ | ||
| config?: ServerAdapterConfig; | ||
| }; | ||
|
|
||
| /** | ||
| * Server status information | ||
| */ | ||
| export type ServerStatus = { | ||
| /** Whether server is running */ | ||
| running: boolean; | ||
|
|
||
| /** Server port */ | ||
| port: number; | ||
|
|
||
| /** Server host */ | ||
| host: string; | ||
|
|
||
| /** Server uptime in milliseconds */ | ||
| uptime: number; | ||
|
|
||
| /** Number of registered routes */ | ||
| routes: number; | ||
|
|
||
| /** Number of registered middleware */ | ||
| middlewares: number; | ||
|
|
||
| /** Current lifecycle state */ | ||
| lifecycleState?: ServerLifecycleState; | ||
|
|
||
| /** Number of active connections */ | ||
| activeConnections?: number; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // Streaming Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * SSE write options | ||
| */ | ||
| export type SSEWriteOptions = { | ||
| /** Event name */ | ||
| event?: string; | ||
|
|
||
| /** Event data (will be JSON stringified if object) */ | ||
| data: string | object; | ||
|
|
||
| /** Event ID */ | ||
| id?: string; | ||
|
|
||
| /** Retry interval in milliseconds */ | ||
| retry?: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Data stream writer interface | ||
| */ | ||
| export type DataStreamWriter = { | ||
| /** Write text start event */ | ||
| writeTextStart(id: string): Promise<void>; | ||
|
|
||
| /** Write text delta event */ | ||
| writeTextDelta(id: string, delta: string): Promise<void>; | ||
|
|
||
| /** Write text end event */ | ||
| writeTextEnd(id: string): Promise<void>; | ||
|
|
||
| /** Write tool call event */ | ||
| writeToolCall(toolCall: { | ||
| id: string; | ||
| name: string; | ||
| arguments: Record<string, unknown>; | ||
| }): Promise<void>; | ||
|
|
||
| /** Write tool result event */ | ||
| writeToolResult(toolResult: { | ||
| id: string; | ||
| name: string; | ||
| result: unknown; | ||
| }): Promise<void>; | ||
|
|
||
| /** Write arbitrary data event */ | ||
| writeData(data: unknown): Promise<void>; | ||
|
|
||
| /** Write error event */ | ||
| writeError(error: { message: string; code?: string }): Promise<void>; | ||
|
|
||
| /** Close the stream */ | ||
| close(): Promise<void>; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // WebSocket Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * WebSocket message types | ||
| */ | ||
| export type WebSocketMessageType = | ||
| | "text" | ||
| | "binary" | ||
| | "ping" | ||
| | "pong" | ||
| | "close"; | ||
|
|
||
| /** | ||
| * WebSocket message | ||
| */ | ||
| export type WebSocketMessage = { | ||
| type: WebSocketMessageType; | ||
| data: string | ArrayBuffer; | ||
| timestamp: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Authenticated user information | ||
| */ | ||
| export type AuthenticatedUser = { | ||
| id: string; | ||
| email?: string; | ||
| name?: string; | ||
| roles?: string[]; | ||
| permissions?: string[]; | ||
| metadata?: Record<string, unknown>; | ||
| }; | ||
|
|
||
| /** | ||
| * WebSocket connection | ||
| */ | ||
| export type WebSocketConnection = { | ||
| id: string; | ||
| socket: unknown; | ||
| user?: AuthenticatedUser; | ||
| metadata: Record<string, unknown>; | ||
| createdAt: number; | ||
| lastActivity: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Authentication strategy types | ||
| */ | ||
| export type AuthStrategy = "bearer" | "apiKey" | "basic" | "custom" | "none"; | ||
|
|
||
| /** | ||
| * Authentication configuration | ||
| */ | ||
| export type AuthConfig = { | ||
| strategy: AuthStrategy; | ||
| required?: boolean; | ||
| headerName?: string; | ||
| queryParam?: string; | ||
| validate?: (token: string) => Promise<AuthenticatedUser | null>; | ||
| roles?: string[]; | ||
| permissions?: string[]; | ||
| }; | ||
|
|
||
| /** | ||
| * WebSocket handler interface | ||
| */ | ||
| export type WebSocketHandler = { | ||
| onOpen?: (connection: WebSocketConnection) => void | Promise<void>; | ||
| onMessage?: ( | ||
| connection: WebSocketConnection, | ||
| message: WebSocketMessage, | ||
| ) => void | Promise<void>; | ||
| onClose?: ( | ||
| connection: WebSocketConnection, | ||
| code: number, | ||
| reason: string, | ||
| ) => void | Promise<void>; | ||
| onError?: ( | ||
| connection: WebSocketConnection, | ||
| error: Error, | ||
| ) => void | Promise<void>; | ||
| }; | ||
|
|
||
| /** | ||
| * WebSocket server configuration | ||
| */ | ||
| export type WebSocketConfig = { | ||
| path?: string; | ||
| maxConnections?: number; | ||
| pingInterval?: number; | ||
| pongTimeout?: number; | ||
| maxMessageSize?: number; | ||
| auth?: AuthConfig; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // Lifecycle Types | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Server lifecycle states | ||
| * Represents the current state of the server adapter | ||
| */ | ||
| export type ServerLifecycleState = | ||
| | "uninitialized" | ||
| | "initializing" | ||
| | "initialized" | ||
| | "starting" | ||
| | "running" | ||
| | "draining" | ||
| | "stopping" | ||
| | "stopped" | ||
| | "error"; | ||
|
|
||
| /** | ||
| * Configuration for graceful shutdown behavior | ||
| */ | ||
| export type ShutdownConfig = { | ||
| /** | ||
| * Maximum time to wait for graceful shutdown in milliseconds | ||
| * Default: 30000 (30 seconds) | ||
| */ | ||
| gracefulShutdownTimeoutMs?: number; | ||
|
|
||
| /** | ||
| * Maximum time to wait for connections to drain in milliseconds | ||
| * Default: 15000 (15 seconds) | ||
| */ | ||
| drainTimeoutMs?: number; | ||
|
|
||
| /** | ||
| * Whether to force close connections after timeout | ||
| * Default: true | ||
| */ | ||
| forceClose?: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Required shutdown configuration (with defaults applied) | ||
| */ | ||
| export type RequiredShutdownConfig = { | ||
| gracefulShutdownTimeoutMs: number; | ||
| drainTimeoutMs: number; | ||
| forceClose: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Tracked connection for graceful shutdown | ||
| */ | ||
| export type TrackedConnection = { | ||
| /** Unique connection identifier */ | ||
| id: string; | ||
|
|
||
| /** Timestamp when connection was created */ | ||
| createdAt: number; | ||
|
|
||
| /** Underlying socket or connection object */ | ||
| socket?: unknown; | ||
|
|
||
| /** Request ID if associated with a request */ | ||
| requestId?: string; | ||
|
|
||
| /** Whether the connection is currently processing a request */ | ||
| isActive?: boolean; | ||
| }; | ||
|
|
||
| // ============================================ | ||
| // Error Types | ||
| // Error Constants (runtime values) |
There was a problem hiding this comment.
src/lib/server/types.ts is now described as a backward-compatibility re-export shim, but the file still contains local runtime exports (e.g., error constants) below this header. Since those same constants are now also defined/exported from src/lib/types/serverTypes.ts, this creates two sources of truth and can lead to drift. Consider making this file a pure re-export (remove local declarations) and keep the canonical definitions only in types/serverTypes.ts.
|
|
||
| **Tech Stack:** TypeScript 5.x, Vitest, pnpm | ||
|
|
||
| **Working directory:** `/Users/sachinsharma/Developer/temp/neurolink-fork/feat/type-fix` |
There was a problem hiding this comment.
This doc hard-codes a developer-specific absolute path (/Users/...) as the working directory. That leaks local environment details (and a username) into the repo and will be incorrect for other contributors. Please replace it with a repo-relative path or remove the line entirely.
Suggested change
| **Working directory:** `/Users/sachinsharma/Developer/temp/neurolink-fork/feat/type-fix` | |
| **Working directory:** repository root (`.`) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/types/serverTypes.ts (1)
278-283: Keep the authenticated user shape consistent across server paths.
AuthConfig.validate()andWebSocketConnection.useralready useAuthenticatedUser, butServerContext.useris a narrower inline subset. That dropsname,permissions, andmetadatafrom HTTP route handlers even when the auth layer has them.Suggested type adjustment
- user?: { - id: string; - email?: string; - roles?: string[]; - }; + user?: AuthenticatedUser;Also applies to: 913-950
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/serverTypes.ts` around lines 278 - 283, ServerContext.user is defined as a narrower inline type and drops fields (name, permissions, metadata) that AuthConfig.validate() and WebSocketConnection.user provide via AuthenticatedUser; update ServerContext.user to use the shared AuthenticatedUser type (or a nullable/optional AuthenticatedUser) so HTTP route handlers receive the full authenticated shape, and update any other inline user declarations (e.g., the similar definitions around the 913-950 region) to reference AuthenticatedUser as well.src/lib/server/types.ts (1)
6-7: Duplicate exports create maintenance fragility.The wildcard re-export at line 7 includes
ErrorCategory,ErrorSeverity,ServerAdapterErrorCode, andServerAdapterErrorContextfromserverTypes.ts, but lines 16-106 locally redefine and re-export them. ES modules resolve this by preferring local exports, so the code works, but both copies must stay in sync.Consider removing the local definitions and relying solely on the wildcard re-export from the canonical source:
♻️ Proposed simplification
/** * Server Adapter Types * Re-exported from types/serverTypes.ts for backward compatibility */ // Re-export all types from canonical location export * from "../types/serverTypes.js"; - -// ============================================ -// Error Constants (runtime values) -// ============================================ - -/** - * Error categories for server adapter errors - */ -export const ErrorCategory = { - CONFIG: "CONFIG", - VALIDATION: "VALIDATION", - EXECUTION: "EXECUTION", - EXTERNAL: "EXTERNAL", - RATE_LIMIT: "RATE_LIMIT", - AUTHENTICATION: "AUTHENTICATION", - AUTHORIZATION: "AUTHORIZATION", - STREAMING: "STREAMING", - WEBSOCKET: "WEBSOCKET", -} as const; - -export type ErrorCategoryType = - (typeof ErrorCategory)[keyof typeof ErrorCategory]; - -/** - * Error severity levels - */ -export const ErrorSeverity = { - LOW: "LOW", - MEDIUM: "MEDIUM", - HIGH: "HIGH", - CRITICAL: "CRITICAL", -} as const; - -export type ErrorSeverityType = - (typeof ErrorSeverity)[keyof typeof ErrorSeverity]; - -/** - * Server adapter error codes - */ -export const ServerAdapterErrorCode = { - // Configuration errors - INVALID_CONFIG: "SERVER_ADAPTER_INVALID_CONFIG", - MISSING_DEPENDENCY: "SERVER_ADAPTER_MISSING_DEPENDENCY", - FRAMEWORK_INIT_FAILED: "SERVER_ADAPTER_FRAMEWORK_INIT_FAILED", - // ... (all other codes) -} as const; - -export type ServerAdapterErrorCodeType = - (typeof ServerAdapterErrorCode)[keyof typeof ServerAdapterErrorCode]; - -/** - * Error context for server adapter errors - */ -export type ServerAdapterErrorContext = { - category: ErrorCategoryType; - severity: ErrorSeverityType; - retryable: boolean; - retryAfterMs?: number; - requestId?: string; - path?: string; - method?: string; - details?: Record<string, unknown>; - cause?: Error; -};Since
src/lib/server/errors.tsimports these from./types.js, it would then receive them via the wildcard re-export fromserverTypes.ts, which contains identical definitions.Also applies to: 16-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/types.ts` around lines 6 - 7, The file duplicates the exported symbols ErrorCategory, ErrorSeverity, ServerAdapterErrorCode, and ServerAdapterErrorContext: remove the local re-definitions of those types/enums in this module and rely solely on the existing wildcard re-export (export * from "../types/serverTypes.js"); ensure you delete the local declarations (the blocks defining ErrorCategory, ErrorSeverity, ServerAdapterErrorCode, ServerAdapterErrorContext) so consumers (e.g., imports from ./types.js like errors.ts) receive the canonical versions via the wildcard export.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-04-03-type-system-consolidation.md`:
- Line 752: The export line currently re-exports from "./ragTypes.ts" which is
inconsistent with the other barrel exports and will break resolution; update the
export statement to use the .js extension (change "./ragTypes.ts" to
"./ragTypes.js") so it matches the rest of the barrel exports (the export * from
"./ragTypes.js" entry).
In `@src/lib/types/serverTypes.ts`:
- Around line 53-57: RequiredServerAdapterConfig currently omits the redaction
field so normalized configs lose or force casts for that setting; add redaction
to the normalized/required config types so defaults include it—specifically
update RequiredServerAdapterConfig to include a redaction: RedactionConfig (or
redaction?: RedactionConfig with documented default) and mirror the same change
for the other "required/with defaults applied" types in this file (search for
other Required*/normalized server config types) so redaction is preserved when
you normalize server config.
---
Nitpick comments:
In `@src/lib/server/types.ts`:
- Around line 6-7: The file duplicates the exported symbols ErrorCategory,
ErrorSeverity, ServerAdapterErrorCode, and ServerAdapterErrorContext: remove the
local re-definitions of those types/enums in this module and rely solely on the
existing wildcard re-export (export * from "../types/serverTypes.js"); ensure
you delete the local declarations (the blocks defining ErrorCategory,
ErrorSeverity, ServerAdapterErrorCode, ServerAdapterErrorContext) so consumers
(e.g., imports from ./types.js like errors.ts) receive the canonical versions
via the wildcard export.
In `@src/lib/types/serverTypes.ts`:
- Around line 278-283: ServerContext.user is defined as a narrower inline type
and drops fields (name, permissions, metadata) that AuthConfig.validate() and
WebSocketConnection.user provide via AuthenticatedUser; update
ServerContext.user to use the shared AuthenticatedUser type (or a
nullable/optional AuthenticatedUser) so HTTP route handlers receive the full
authenticated shape, and update any other inline user declarations (e.g., the
similar definitions around the 913-950 region) to reference AuthenticatedUser as
well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de57a431-83b7-4416-a9ed-adfcc4ee7f1e
📒 Files selected for processing (21)
docs/superpowers/plans/2026-04-03-type-system-consolidation.mdsrc/lib/client/auth.tssrc/lib/client/httpClient.tssrc/lib/client/index.tssrc/lib/client/interceptors.tssrc/lib/client/reactHooks.tsxsrc/lib/client/sseClient.tssrc/lib/client/streamingClient.tssrc/lib/client/wsClient.tssrc/lib/rag/types.tssrc/lib/server/types.tssrc/lib/types/analytics.tssrc/lib/types/cli.tssrc/lib/types/clientTypes.tssrc/lib/types/configTypes.tssrc/lib/types/index.tssrc/lib/types/ragTypes.tssrc/lib/types/sdkTypes.tssrc/lib/types/serverTypes.tssrc/lib/types/streamTypes.tssrc/lib/types/typeAliases.ts
|
|
||
| // AI subsystems | ||
| export * from "./mcpTypes.js"; | ||
| export * from "./ragTypes.ts"; |
There was a problem hiding this comment.
Typo: Use .js extension for consistency.
All other export paths in this barrel example use .js extensions, but this line uses .ts. This would cause a module resolution error.
-export * from "./ragTypes.ts";
+export * from "./ragTypes.js";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export * from "./ragTypes.ts"; | |
| export * from "./ragTypes.js"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/plans/2026-04-03-type-system-consolidation.md` at line 752,
The export line currently re-exports from "./ragTypes.ts" which is inconsistent
with the other barrel exports and will break resolution; update the export
statement to use the .js extension (change "./ragTypes.ts" to "./ragTypes.js")
so it matches the rest of the barrel exports (the export * from "./ragTypes.js"
entry).
Contributor
Author
Review Feedback AddressedChanges Made
Additional changes in this cycle
Validation
Requesting Re-reviewAll feedback addressed. Please re-review. |
Contributor
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
Triggering a fresh review now. 🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/types/clientTypes.ts (1)
45-58:⚠️ Potential issue | 🟠 MajorPreserve deprecated aliases in
@neurolink/client/types.Line 8 documents this file as
@neurolink/client/types, so renaming the exported symbols here removes the oldRetryConfig,ApiResponse,ProviderStatus,StreamEvent,StreamResult,MiddlewareContext,ChatMessage,OAuth2Config, andTokenRefreshResultnames for direct subpath consumers. The collision fix insrc/lib/types/index.tsis fine, but this module should keep deprecated aliases until the next major.♻️ Compatibility pattern
+/** `@deprecated` Use ClientRetryConfig */ +export type RetryConfig = ClientRetryConfig; + +/** `@deprecated` Use ClientApiResponse */ +export type ApiResponse<T> = ClientApiResponse<T>;Repeat the same alias pattern for the other renamed public symbols in this file.
Also applies to: 81-92, 115-131, 153-176, 203-220, 500-509, 535-550, 1095-1122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/clientTypes.ts` around lines 45 - 58, The file removed deprecated exported type names used by consumers; restore backward-compatible aliases by re-exporting the old names (e.g., add aliases such as export type RetryConfig = ClientRetryConfig and similarly export ApiResponse = ClientApiResponse, ProviderStatus = ClientProviderStatus, StreamEvent = ClientStreamEvent, StreamResult = ClientStreamResult, MiddlewareContext = ClientMiddlewareContext, ChatMessage = ClientChatMessage, OAuth2Config = ClientOAuth2Config, TokenRefreshResult = ClientTokenRefreshResult) for each renamed symbol blocks noted (also apply the same pattern for the other ranges referenced), mark them as deprecated in comments, and keep the primary new names (Client*) as the canonical exports so consumers of `@neurolink/client/types` using the old identifiers continue to work until the next major.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/types/index.ts`:
- Around line 295-296: The wildcard export from serverTypes.js is being shadowed
by the earlier explicit export of MCP's RateLimitConfig; to fix this, stop using
export * and instead explicitly re-export the server adapter types from
serverTypes.js, e.g. export { RateLimitConfig as ServerRateLimitConfig,
OtherServerType1, OtherServerType2 } from "./serverTypes.js" so the server
RateLimitConfig is available under a distinct name and other server symbols are
preserved; reference the symbols serverTypes.js, RateLimitConfig, and
mcpTypes.js when making the change.
In `@src/lib/types/serverTypes.ts`:
- Around line 292-296: ServerContext.user is declared inline as { id, email?,
roles? } which narrows and diverges from the canonical AuthenticatedUser;
replace that inline type with the exported AuthenticatedUser interface so the
full contract (name, permissions, metadata, etc.) is preserved. Update
src/lib/types/serverTypes.ts to import AuthenticatedUser from
src/lib/types/authTypes.ts and change the user property type to user?:
AuthenticatedUser; remove any duplicated auth fields in this file to avoid a
second drifting auth model.
---
Outside diff comments:
In `@src/lib/types/clientTypes.ts`:
- Around line 45-58: The file removed deprecated exported type names used by
consumers; restore backward-compatible aliases by re-exporting the old names
(e.g., add aliases such as export type RetryConfig = ClientRetryConfig and
similarly export ApiResponse = ClientApiResponse, ProviderStatus =
ClientProviderStatus, StreamEvent = ClientStreamEvent, StreamResult =
ClientStreamResult, MiddlewareContext = ClientMiddlewareContext, ChatMessage =
ClientChatMessage, OAuth2Config = ClientOAuth2Config, TokenRefreshResult =
ClientTokenRefreshResult) for each renamed symbol blocks noted (also apply the
same pattern for the other ranges referenced), mark them as deprecated in
comments, and keep the primary new names (Client*) as the canonical exports so
consumers of `@neurolink/client/types` using the old identifiers continue to work
until the next major.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c1bea962-823d-48e0-b58e-886c1164646e
📒 Files selected for processing (20)
src/lib/client/auth.tssrc/lib/client/httpClient.tssrc/lib/client/index.tssrc/lib/client/interceptors.tssrc/lib/client/reactHooks.tsxsrc/lib/client/sseClient.tssrc/lib/client/streamingClient.tssrc/lib/client/wsClient.tssrc/lib/rag/types.tssrc/lib/server/types.tssrc/lib/types/analytics.tssrc/lib/types/cli.tssrc/lib/types/clientTypes.tssrc/lib/types/configTypes.tssrc/lib/types/index.tssrc/lib/types/ragTypes.tssrc/lib/types/sdkTypes.tssrc/lib/types/serverTypes.tssrc/lib/types/streamTypes.tssrc/lib/types/typeAliases.ts
✅ Files skipped from review due to trivial changes (8)
- src/lib/client/interceptors.ts
- src/lib/types/cli.ts
- src/lib/types/streamTypes.ts
- src/lib/types/sdkTypes.ts
- src/lib/types/configTypes.ts
- src/lib/client/streamingClient.ts
- src/lib/client/wsClient.ts
- src/lib/client/sseClient.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/lib/client/reactHooks.tsx
- src/lib/types/analytics.ts
- src/lib/client/auth.ts
- src/lib/server/types.ts
- src/lib/client/index.ts
- src/lib/types/typeAliases.ts
- src/lib/client/httpClient.ts
Comment on lines
+292
to
+296
| user?: { | ||
| id: string; | ||
| email?: string; | ||
| roles?: string[]; | ||
| }; |
There was a problem hiding this comment.
Avoid a second, drifting auth model here.
Line 960 returns AuthenticatedUser, but ServerContext.user at Lines 292-296 narrows that back to an inline { id, email, roles }. That drops name, permissions, and metadata from the main request contract, and it also leaves these new auth types living outside the canonical auth module.
💡 Minimal fix for the immediate drift
- user?: {
- id: string;
- email?: string;
- roles?: string[];
- };
+ user?: AuthenticatedUser;Based on learnings: In juspay/neurolink, the canonical location for auth-related TypeScript types is src/lib/types/authTypes.ts.
Also applies to: 926-963
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/types/serverTypes.ts` around lines 292 - 296, ServerContext.user is
declared inline as { id, email?, roles? } which narrows and diverges from the
canonical AuthenticatedUser; replace that inline type with the exported
AuthenticatedUser interface so the full contract (name, permissions, metadata,
etc.) is preserved. Update src/lib/types/serverTypes.ts to import
AuthenticatedUser from src/lib/types/authTypes.ts and change the user property
type to user?: AuthenticatedUser; remove any duplicated auth fields in this file
to avoid a second drifting auth model.
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
…circular deps, absorb server types Phase 1 — Name collision resolution: - Rename 10 colliding types in clientTypes.ts with Client prefix (ClientStreamResult, ClientRetryConfig, ClientChatMessage, etc.) - Remove 5 duplicate types from typeAliases.ts (canonical defs in common.ts) - Rename ErrorInfo → AnalyticsErrorInfo in analytics.ts - Update barrel index.ts and all client/* consumer files Phase 2 — Circular dependency breaks: - Fix cli.ts/streamTypes.ts: import EvaluationData from ./evaluation.js instead of ../index.js (breaks circular through barrel) - Change configTypes.ts MCPToolRegistry to import type (breaks runtime dep) - Move RAGConfig to types/ragTypes.ts, convert rag/types.ts to re-export Phase 3 — Rogue type file absorption: - Create types/serverTypes.ts with all 54 server adapter type definitions - Convert server/types.ts to re-export shim for backward compatibility - Add serverTypes.ts to barrel exports Verified: 0 type errors, lint clean, full build passing, public API intact.
Contributor
Author
Review Feedback Addressed (Cycle 2)Changes Made
Validation
@coderabbitai review |
|
Triggering a fresh review now. 🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/client/httpClient.ts (1)
488-511:⚠️ Potential issue | 🟠 Major
stream()does not enforce the client timeout path used byrequest().At Line 496,
stream()callsthis.config.fetch(...)directly with onlyrequestOptions?.signal, so default timeout handling is skipped and long-lived hangs are possible.As per coding guidelines:
**/*.ts: UsewithTimeoututility to wrap async calls for error handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/httpClient.ts` around lines 488 - 511, The stream() method is calling this.config.fetch(...) directly and thus bypasses the client timeout logic used by request(); wrap the fetch call with the withTimeout utility (same pattern as request()) so the fetch is cancelled/throws on timeout, passing through requestOptions?.signal and the configured timeout value (e.g., this.config.timeout or timeoutMs) and surface the timeout error consistently; update stream() to import/use withTimeout and call withTimeout(() => this.config.fetch(...), timeoutValue, { signal: requestOptions?.signal }) (preserving headers/body) so the stream path enforces the same timeout behavior as request().
♻️ Duplicate comments (1)
src/lib/types/serverTypes.ts (1)
922-959:⚠️ Potential issue | 🟠 MajorRe-export the canonical
AuthenticatedUserinstead of redefining it.
AuthenticatedUseris still declared locally inserverTypes.ts, and that local copy now feedsServerContext.user,WebSocketConnection.user, andAuthConfig.validate. Please re-export the shared auth user type from./authTypes.jshere rather than maintaining a second auth contract in the server module.Based on learnings: In juspay/neurolink, the canonical location for auth-related TypeScript types is
src/lib/types/authTypes.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/serverTypes.ts` around lines 922 - 959, Replace the locally declared AuthenticatedUser with the canonical type by importing and re-exporting the shared type (AuthenticatedUser) from the central auth types module, then remove the local AuthenticatedUser declaration; ensure all references such as WebSocketConnection.user, ServerContext.user, and AuthConfig.validate continue to use the imported AuthenticatedUser type so the module exports the same canonical type rather than a duplicate.
🧹 Nitpick comments (1)
src/lib/client/httpClient.ts (1)
539-540: Avoidunknown[]+ type assertions when assemblingClientStreamResult.At Line 539/540 and Line 559/560 (also Line 592/593), accumulating into
unknown[]and casting later can hide malformed tool payloads. Prefer strongly-typed accumulators (e.g.,NonNullable<ClientStreamResult["toolCalls"]>/NonNullable<ClientStreamResult["toolResults"]>) and return without assertions.Also applies to: 557-560, 592-593
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/httpClient.ts` around lines 539 - 540, Replace the loose unknown[] accumulators with strongly typed arrays matching ClientStreamResult fields (e.g., declare toolCalls as NonNullable<ClientStreamResult["toolCalls"]> and toolResults as NonNullable<ClientStreamResult["toolResults"]>), update any pushes/assignments to preserve those types (validate or skip malformed payloads instead of casting), and return a properly typed ClientStreamResult without using type assertions; apply the same change to the other accumulator declarations that gather tool payloads so all uses validate shape and keep types authoritative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lib/client/httpClient.ts`:
- Around line 488-511: The stream() method is calling this.config.fetch(...)
directly and thus bypasses the client timeout logic used by request(); wrap the
fetch call with the withTimeout utility (same pattern as request()) so the fetch
is cancelled/throws on timeout, passing through requestOptions?.signal and the
configured timeout value (e.g., this.config.timeout or timeoutMs) and surface
the timeout error consistently; update stream() to import/use withTimeout and
call withTimeout(() => this.config.fetch(...), timeoutValue, { signal:
requestOptions?.signal }) (preserving headers/body) so the stream path enforces
the same timeout behavior as request().
---
Duplicate comments:
In `@src/lib/types/serverTypes.ts`:
- Around line 922-959: Replace the locally declared AuthenticatedUser with the
canonical type by importing and re-exporting the shared type (AuthenticatedUser)
from the central auth types module, then remove the local AuthenticatedUser
declaration; ensure all references such as WebSocketConnection.user,
ServerContext.user, and AuthConfig.validate continue to use the imported
AuthenticatedUser type so the module exports the same canonical type rather than
a duplicate.
---
Nitpick comments:
In `@src/lib/client/httpClient.ts`:
- Around line 539-540: Replace the loose unknown[] accumulators with strongly
typed arrays matching ClientStreamResult fields (e.g., declare toolCalls as
NonNullable<ClientStreamResult["toolCalls"]> and toolResults as
NonNullable<ClientStreamResult["toolResults"]>), update any pushes/assignments
to preserve those types (validate or skip malformed payloads instead of
casting), and return a properly typed ClientStreamResult without using type
assertions; apply the same change to the other accumulator declarations that
gather tool payloads so all uses validate shape and keep types authoritative.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31f1bcfa-3324-4fb6-b894-09b2e8884282
📒 Files selected for processing (36)
src/cli/commands/authProviders.tssrc/cli/commands/evaluate.tssrc/cli/commands/mcp.d.tssrc/cli/commands/mcp.tssrc/cli/commands/models.tssrc/cli/commands/task.tssrc/cli/commands/workflow.tssrc/cli/errorHandler.tssrc/cli/factories/commandFactory.tssrc/cli/factories/sagemakerCommandFactory.tssrc/cli/loop/conversationSelector.tssrc/cli/loop/optionsSchema.tssrc/cli/loop/session.tssrc/cli/utils/audioFileUtils.tssrc/cli/utils/envManager.tssrc/cli/utils/videoFileUtils.tssrc/lib/client/auth.tssrc/lib/client/httpClient.tssrc/lib/client/index.tssrc/lib/client/interceptors.tssrc/lib/client/reactHooks.tsxsrc/lib/client/sseClient.tssrc/lib/client/streamingClient.tssrc/lib/client/wsClient.tssrc/lib/rag/types.tssrc/lib/server/types.tssrc/lib/types/analytics.tssrc/lib/types/cli.tssrc/lib/types/clientTypes.tssrc/lib/types/configTypes.tssrc/lib/types/index.tssrc/lib/types/ragTypes.tssrc/lib/types/sdkTypes.tssrc/lib/types/serverTypes.tssrc/lib/types/streamTypes.tssrc/lib/types/typeAliases.ts
✅ Files skipped from review due to trivial changes (27)
- src/cli/commands/authProviders.ts
- src/cli/loop/conversationSelector.ts
- src/cli/commands/mcp.d.ts
- src/cli/utils/audioFileUtils.ts
- src/cli/commands/workflow.ts
- src/cli/errorHandler.ts
- src/cli/utils/envManager.ts
- src/cli/loop/optionsSchema.ts
- src/lib/types/cli.ts
- src/cli/commands/task.ts
- src/lib/types/streamTypes.ts
- src/cli/commands/evaluate.ts
- src/lib/types/sdkTypes.ts
- src/lib/types/configTypes.ts
- src/cli/loop/session.ts
- src/cli/commands/models.ts
- src/lib/client/interceptors.ts
- src/lib/client/reactHooks.tsx
- src/cli/factories/sagemakerCommandFactory.ts
- src/lib/client/streamingClient.ts
- src/lib/types/analytics.ts
- src/lib/client/wsClient.ts
- src/lib/client/sseClient.ts
- src/cli/factories/commandFactory.ts
- src/cli/utils/videoFileUtils.ts
- src/lib/client/index.ts
- src/cli/commands/mcp.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/lib/rag/types.ts
- src/lib/client/auth.ts
- src/lib/types/ragTypes.ts
- src/lib/types/typeAliases.ts
- src/lib/types/clientTypes.ts
- src/lib/types/index.ts
Contributor
|
🎉 This PR is included in version 9.44.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Apr 9, 2026
7 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/lib/types/— rename 10 colliding types inclientTypes.tswithClientprefix, deduplicate 5 types fromtypeAliases.ts, renameErrorInfo→AnalyticsErrorInfoinanalytics.tstypes/→ implementation modules — fixcli.ts/streamTypes.tscircular import throughindex.js, changeconfigTypes.tsto useimport typeforMCPToolRegistry, moveRAGConfigtotypes/ragTypes.tsserver/types.ts(54 type definitions) into canonicaltypes/serverTypes.ts, convert original to re-export shimChanges by commit
Verification
tsc --noEmit: 0 errorstsc --noEmit --strict: 0 errorspnpm run lint: 0 errors (1 pre-existing warning)pnpm run build: passes (SDK + CLI + browser + publint)prettier --check: all files cleanimport type)src/lib/index.ts) unchangedWhat was deferred
rag/types.tsconsolidation — incompatibleRerankerConfigshapes betweenrag/types.tsandragTypes.tsprocessors/base/types.tsconsolidation — import path issues with../errors/FileErrorCode.jsTest plan
tsc --noEmitand--strict)pnpm run build)pnpm run lint)prettier --check)Summary by CodeRabbit