Repository navigation
feat(client): add client SDKs with HTTP client, React hooks, and AI SDK adapter - #896
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces a complete Client SDK for NeuroLink, featuring a typed HTTP client with middleware support, React hooks for chat/agents/workflows/voice/streaming/tools, OAuth2 and JWT authentication managers, comprehensive error handling, SSE and WebSocket streaming clients, Vercel AI SDK integration, and extensive examples and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as React Component
participant Provider as NeuroLinkProvider
participant Middleware as Middleware Chain
participant HTTP as HTTP Client
participant Server as NeuroLink Server
Client->>Provider: useChat(options)
Provider->>Provider: createClient(config)
Client->>HTTP: submit message
HTTP->>Middleware: request(message)
Middleware->>Middleware: api-key-auth
Middleware->>Middleware: logging
Middleware->>Middleware: retry-logic
Middleware->>HTTP: continue
HTTP->>Server: POST /api/chat
Server-->>HTTP: SSE response (text/event-stream)
HTTP->>HTTP: parse StreamEvent
HTTP->>Client: onText callback
HTTP->>Client: onToolCall callback
HTTP->>Client: onDone callback
Client->>Client: update chat state
Client->>Client: render messages
sequenceDiagram
participant App as Next.js App
participant Adapter as NeuroLinkProvider
participant Vercel as Vercel AI SDK
participant Client as NeuroLink Client
participant Server as LLM Server
App->>Adapter: createNeuroLinkProvider(config)
App->>Vercel: generateText({model: neurolink(...), prompt})
Vercel->>Adapter: doGenerate(options)
Adapter->>Client: generate(prompt, config)
Client->>Server: POST /api/generate
Server-->>Client: GenerateResponse
Client->>Adapter: {text, finishReason, usage}
Adapter->>Vercel: LanguageModelResponse
Vercel->>App: {text, usage}
sequenceDiagram
participant Browser as Browser
participant SSEClient as SSEClient
participant Fetch as fetch() API
participant Server as SSE Endpoint
participant Reconnect as Auto-Reconnect
Browser->>SSEClient: stream(path, callbacks)
SSEClient->>Fetch: POST /api/stream
Fetch->>Server: connect
Server-->>Fetch: 200 OK (text/event-stream)
Fetch-->>SSEClient: ReadableStream
Server-->>SSEClient: data: {type: "text", content: "..."}
SSEClient->>Browser: onText callback
Server-->>SSEClient: data: {type: "tool-call", name: "..."}
SSEClient->>Browser: onToolCall callback
Server-->>SSEClient: data: "[DONE]"
SSEClient->>Browser: onDone callback
alt Network Error
Fetch-->>SSEClient: Error
SSEClient->>SSEClient: autoReconnect enabled
SSEClient->>Reconnect: exponential backoff
Reconnect->>Fetch: retry connection
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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
✅ 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 |
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
This PR introduces a full client-side SDK surface (HTTP client + streaming clients + React hooks + Vercel AI SDK adapter), wires up public exports/types, and updates the Hono SSE adapter so streaming output matches the client SDK’s expected StreamEvent + [DONE] termination format.
Changes:
- Add new
src/lib/client/*client SDK implementation (HTTP client, auth utilities, interceptors, SSE/WS streaming clients, AI SDK adapter, React hooks) and canonical client SDK types insrc/lib/types/clientTypes.ts. - Export client SDK APIs/types from the root SDK entrypoints and package subpath (
./client), and enable TSX builds viajsx: react-jsx. - Update server-side Hono SSE streaming to emit
StreamEvent-shaped payloads and[DONE]termination.
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Enables react-jsx so .tsx client hook code can typecheck/build. |
| tsconfig.cli.json | Extends CLI build to support .tsx under src/lib/**. |
| src/lib/types/index.ts | Re-exports client SDK types with aliases to reduce collisions. |
| src/lib/types/clientTypes.ts | Adds canonical client SDK type surface (config, streaming, hooks, auth, AI adapter). |
| src/lib/server/adapters/honoAdapter.ts | Adjusts SSE streaming payloads to align with client SDK expectations. |
| src/lib/client/aiSdkAdapter.ts | Adds a Vercel AI SDK-compatible adapter (LanguageModel-style). |
| src/lib/client/interceptors.ts | Adds composable middleware (auth/logging/retry/rate-limit/cache/timeout/error handling). |
| src/lib/client/sseClient.ts | Adds a dedicated SSE streaming client implementation. |
| src/lib/client/auth.ts | Adds OAuth2/JWT token managers and JWT helpers. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+720
to
+747
| export function createTimeoutInterceptor( | ||
| options: TimeoutInterceptorOptions, | ||
| ): Middleware { | ||
| const { timeout, onTimeout } = options; | ||
|
|
||
| return async (request, next) => { | ||
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => { | ||
| controller.abort(); | ||
| onTimeout?.(request); | ||
| }, timeout); | ||
|
|
||
| try { | ||
| // Store original abort handling | ||
| request.context.timeoutSignal = controller.signal; | ||
| const response = await next(); | ||
| clearTimeout(timeoutId); | ||
| return response; | ||
| } catch (error) { | ||
| clearTimeout(timeoutId); | ||
| if ((error as Error).name === "AbortError") { | ||
| throw new Error(`Request timed out after ${timeout}ms`, { | ||
| cause: error, | ||
| }); | ||
| } | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
createTimeoutInterceptor() sets request.context.timeoutSignal, but the HTTP client request execution doesn’t read/use this field, so the interceptor won’t actually abort requests. Either (a) have the HTTP client combine options.signal with request.context.timeoutSignal when calling fetch, or (b) implement the timeout by racing next() against a timer and aborting via an AbortController that is actually wired into the request.
Comment on lines
+119
to
+170
| constructor(config: SSEConfig) { | ||
| this.config = { | ||
| baseUrl: config.baseUrl, | ||
| apiKey: config.apiKey ?? "", | ||
| token: config.token ?? "", | ||
| timeout: config.timeout ?? 30000, | ||
| headers: config.headers ?? {}, | ||
| autoReconnect: config.autoReconnect ?? true, | ||
| maxReconnectAttempts: config.maxReconnectAttempts ?? 5, | ||
| reconnectDelay: config.reconnectDelay ?? 1000, | ||
| maxReconnectDelay: config.maxReconnectDelay ?? 30000, | ||
| useNativeEventSource: config.useNativeEventSource ?? false, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Get current connection state | ||
| */ | ||
| getState(): SSEState { | ||
| return this.state; | ||
| } | ||
|
|
||
| /** | ||
| * Check if connected | ||
| */ | ||
| isConnected(): boolean { | ||
| return this.state === "connected"; | ||
| } | ||
|
|
||
| /** | ||
| * Stream from an endpoint using SSE | ||
| */ | ||
| async stream( | ||
| path: string, | ||
| options: SSERequestOptions = {}, | ||
| callbacks: StreamCallbacks = {}, | ||
| ): Promise<void> { | ||
| this.setState("connecting"); | ||
| this.abortController = new AbortController(); | ||
|
|
||
| const url = this.buildUrl(path); | ||
| const headers = this.buildHeaders(options.headers); | ||
| logger.debug(`[NeuroLinkSSE] Connecting to ${url}`); | ||
|
|
||
| try { | ||
| const response = await fetch(url, { | ||
| method: options.body ? "POST" : "GET", | ||
| headers, | ||
| body: options.body ? JSON.stringify(options.body) : undefined, | ||
| signal: options.signal ?? this.abortController.signal, | ||
| }); | ||
|
|
There was a problem hiding this comment.
SSE client config includes timeout and useNativeEventSource, but neither is currently applied: requests can hang indefinitely, and the native EventSource path is never used. Either implement these options (timeout via AbortController timer; native EventSource when enabled) or remove them to avoid a misleading API surface.
Comment on lines
+163
to
+169
| try { | ||
| const response = await fetch(url, { | ||
| method: options.body ? "POST" : "GET", | ||
| headers, | ||
| body: options.body ? JSON.stringify(options.body) : undefined, | ||
| signal: options.signal ?? this.abortController.signal, | ||
| }); |
There was a problem hiding this comment.
NeuroLinkSSE.stream() always uses the global fetch instead of the ClientConfig.fetch override. This breaks usage in environments without a native fetch (or where a custom fetch is required, e.g., proxies/instrumentation). Consider accepting/using config.fetch similar to NeuroLinkClient (defaulting to globalThis.fetch).
Comment on lines
+512
to
+515
| // Unknown shape — wrap as data event | ||
| return { | ||
| type: "data", | ||
| data: chunk, |
There was a problem hiding this comment.
toStreamEvent() returns { type: "data", data: chunk } for unknown chunk shapes, but StreamEventType (in clientTypes.ts) does not include "data" and the SSE client switch won’t dispatch it. Consider mapping unknown chunks to a supported event type (e.g., metadata with metadata: { raw: chunk }) or extending the shared StreamEventType to include "data" so clients can handle it explicitly.
Suggested change
| // Unknown shape — wrap as data event | |
| return { | |
| type: "data", | |
| data: chunk, | |
| // Unknown shape — wrap as metadata event so clients can still consume it | |
| return { | |
| type: "metadata", | |
| metadata: { raw: chunk }, |
Comment on lines
+431
to
+444
| // Error result, send as error event | ||
| const errorResult = result as { | ||
| error?: { message?: string; code?: string }; | ||
| }; | ||
| await stream.writeSSE({ | ||
| data: JSON.stringify({ | ||
| type: "error", | ||
| error: { | ||
| code: errorResult.error?.code ?? "STREAM_ERROR", | ||
| message: errorResult.error?.message ?? "Stream error", | ||
| }, | ||
| timestamp: Date.now(), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
SSE error events emitted here don’t include the status field required by the client-side ApiError type (you only send { code, message }). This can lead to inconsistent error handling on the client. Include a numeric HTTP status (e.g., errorResult.httpStatus ?? 500 / 500) when building the error payload.
Comment on lines
446
to
448
| // Single result, send as complete event | ||
| await stream.writeSSE({ | ||
| data: JSON.stringify(result), |
There was a problem hiding this comment.
In the non-iterable success path, the code writes JSON.stringify(result) directly. If result is not already a StreamEvent (no type), the client SSE parser will JSON.parse it but then ignore it because event.type is missing/unknown. Consider wrapping single results into a valid StreamEvent (e.g., done or text) or running them through toStreamEvent() as well.
Suggested change
| // Single result, send as complete event | |
| await stream.writeSSE({ | |
| data: JSON.stringify(result), | |
| // Single result, normalize to StreamEvent and send as complete event | |
| const streamEvent = this.toStreamEvent(result); | |
| await stream.writeSSE({ | |
| data: JSON.stringify(streamEvent), |
Comment on lines
+152
to
+197
| let fullText = ""; | ||
|
|
||
| try { | ||
| const result = await self.client.stream( | ||
| { | ||
| input: { text: inputText }, | ||
| provider: self.provider, | ||
| model: self.modelId, | ||
| temperature: temperature ?? self.options.temperature, | ||
| maxTokens: maxTokens ?? self.options.maxTokens, | ||
| systemPrompt, | ||
| }, | ||
| { | ||
| onText: (text) => { | ||
| fullText += text; | ||
| }, | ||
| }, | ||
| { | ||
| signal: abortSignal, | ||
| }, | ||
| ); | ||
|
|
||
| // Yield the accumulated text as a single delta | ||
| if (fullText) { | ||
| yield { | ||
| type: "text-delta" as const, | ||
| textDelta: fullText, | ||
| }; | ||
| } | ||
|
|
||
| // Yield finish event | ||
| yield { | ||
| type: "finish" as const, | ||
| finishReason: self.mapFinishReason(result.finishReason), | ||
| usage: result.usage | ||
| ? { | ||
| promptTokens: result.usage.promptTokens, | ||
| completionTokens: result.usage.completionTokens, | ||
| } | ||
| : undefined, | ||
| }; | ||
| } catch { | ||
| yield { | ||
| type: "finish" as const, | ||
| finishReason: "error", | ||
| }; |
There was a problem hiding this comment.
doStream() buffers the entire response into fullText and yields a single text-delta only after streaming completes, which defeats incremental streaming expected by the AI SDK (streamText consumers won’t receive partial deltas). Consider implementing an async-queue/AsyncIterable that yields each delta as onText fires (or reuse the SDK’s createAsyncStream helper) and then yield the final finish event.
Suggested change
| let fullText = ""; | |
| try { | |
| const result = await self.client.stream( | |
| { | |
| input: { text: inputText }, | |
| provider: self.provider, | |
| model: self.modelId, | |
| temperature: temperature ?? self.options.temperature, | |
| maxTokens: maxTokens ?? self.options.maxTokens, | |
| systemPrompt, | |
| }, | |
| { | |
| onText: (text) => { | |
| fullText += text; | |
| }, | |
| }, | |
| { | |
| signal: abortSignal, | |
| }, | |
| ); | |
| // Yield the accumulated text as a single delta | |
| if (fullText) { | |
| yield { | |
| type: "text-delta" as const, | |
| textDelta: fullText, | |
| }; | |
| } | |
| // Yield finish event | |
| yield { | |
| type: "finish" as const, | |
| finishReason: self.mapFinishReason(result.finishReason), | |
| usage: result.usage | |
| ? { | |
| promptTokens: result.usage.promptTokens, | |
| completionTokens: result.usage.completionTokens, | |
| } | |
| : undefined, | |
| }; | |
| } catch { | |
| yield { | |
| type: "finish" as const, | |
| finishReason: "error", | |
| }; | |
| // Async queue to deliver events to the consumer as they arrive | |
| const queue: any[] = []; | |
| const waiters: Array<() => void> = []; | |
| let closed = false; | |
| const notify = () => { | |
| while (waiters.length > 0) { | |
| const resolve = waiters.shift(); | |
| if (resolve) { | |
| resolve(); | |
| } | |
| } | |
| }; | |
| const push = (item: any) => { | |
| queue.push(item); | |
| notify(); | |
| }; | |
| // Start the underlying client stream in the background so we can | |
| // yield incremental deltas from this async generator. | |
| (async () => { | |
| try { | |
| const result = await self.client.stream( | |
| { | |
| input: { text: inputText }, | |
| provider: self.provider, | |
| model: self.modelId, | |
| temperature: temperature ?? self.options.temperature, | |
| maxTokens: maxTokens ?? self.options.maxTokens, | |
| systemPrompt, | |
| }, | |
| { | |
| onText: (text) => { | |
| if (text) { | |
| push({ | |
| type: "text-delta" as const, | |
| textDelta: text, | |
| }); | |
| } | |
| }, | |
| }, | |
| { | |
| signal: abortSignal, | |
| }, | |
| ); | |
| // Yield finish event once the underlying stream has completed | |
| push({ | |
| type: "finish" as const, | |
| finishReason: self.mapFinishReason(result.finishReason), | |
| usage: result.usage | |
| ? { | |
| promptTokens: result.usage.promptTokens, | |
| completionTokens: result.usage.completionTokens, | |
| } | |
| : undefined, | |
| }); | |
| } catch { | |
| // On error, emit a finish event with error reason | |
| push({ | |
| type: "finish" as const, | |
| finishReason: "error", | |
| }); | |
| } finally { | |
| closed = true; | |
| notify(); | |
| } | |
| })(); | |
| // Async generator loop: yield items from the queue as they arrive | |
| while (!closed || queue.length > 0) { | |
| if (queue.length === 0) { | |
| await new Promise<void>((resolve) => { | |
| waiters.push(resolve); | |
| }); | |
| continue; | |
| } | |
| const item = queue.shift(); | |
| if (item !== undefined) { | |
| yield item; | |
| } |
Comment on lines
+480
to
+492
| export function decodeJWTPayload(token: string): Record<string, unknown> { | ||
| try { | ||
| const parts = token.split("."); | ||
| if (parts.length !== 3) { | ||
| throw new Error("Invalid JWT format"); | ||
| } | ||
|
|
||
| const payload = parts[1]; | ||
| const decoded = atob(payload.replace(/-/g, "+").replace(/_/g, "/")); | ||
| return JSON.parse(decoded); | ||
| } catch { | ||
| throw new Error("Failed to decode JWT token"); | ||
| } |
There was a problem hiding this comment.
decodeJWTPayload() uses atob() directly on a base64url string without adding required padding, and atob is not available in all Node.js runtimes. This can make JWT decoding fail unexpectedly in Node environments. Consider a base64url decoder that normalizes padding and uses globalThis.atob when available, otherwise falls back to Buffer.from(payload, "base64").
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
64-79:⚠️ Potential issue | 🟠 MajorEnsure the new client suite runs as part of the default test gate.
The
test:ci,quality:all,pre-push, andreleasescripts all route throughpnpm run test, which executes onlycontinuous-test-suite.ts. The newtest:clientscript (pointing tocontinuous-test-suite-client.ts) is defined but exists as a parallel, standalone test—it is not invoked by the umbrella suite. This means the new client SDK code is outside the normal shipping gate and will not be tested by default CI runs.Either compose the client tests into the umbrella suite or update the default test gate to include both suites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 64 - 79, The default CI test gate doesn't run the new client suite; update the umbrella test invocation so the client tests run under the default gate—either (A) compose continuous-test-suite-client.ts into the master umbrella test runner (continuous-test-suite.ts) by importing/executing the client suite from within the existing runner, or (B) change the test aggregation script (what "pnpm run test" triggers / the "test:ci" / "test" script) to invoke both continuous-test-suite.ts and continuous-test-suite-client.ts (or run the named scripts "test:client" alongside the existing suite) so that test:client is executed by the CI gate. Ensure you modify the script entries referencing test:ci / test / pnpm run test accordingly and keep naming consistent with the existing script keys (test:client, test:ci, test).
🟡 Minor comments (7)
src/lib/server/adapters/honoAdapter.ts-512-517 (1)
512-517:⚠️ Potential issue | 🟡 MinorDon't emit the unsupported
"data"event type.
StreamEventTypeinsrc/lib/types/clientTypes.ts:140-148does not include"data", andsrc/lib/client/sseClient.ts:472-507has no handler for it. Any chunk that falls through here is silently dropped unless the client contract is extended on both sides in the same PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/adapters/honoAdapter.ts` around lines 512 - 517, The code in honoAdapter.ts currently emits an unsupported event object with type "data" for unknown chunk shapes; instead, stop emitting that unsupported "data" event (which is not part of StreamEventType) — change the fallback to return undefined/null (or throw) so the caller drops the chunk, and add a debug/processLogger.warn indicating an unexpected chunk shape; ensure the function's return type allows undefined and that the caller filters out falsy results, and reference StreamEventType and the SSE client handlers in src/lib/client/sseClient.ts to confirm no "data" handling is required.test/continuous-test-suite-client.ts-289-304 (1)
289-304:⚠️ Potential issue | 🟡 MinorFail this test when the completion event is missing.
This branch still passes as long as any text arrives. If the stream regresses and stops emitting
[DONE]/onDone, the suite will stay green and miss the SSE completion fix this PR is supposed to protect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-client.ts` around lines 289 - 304, The current post-stream check in the test (variables result, content, textChunks, doneReceived, and function logTest) marks the test PASS as long as any text arrived, but it must also ensure the SSE completion event was received; update the logic so that if doneReceived is false you call logTest with a FAIL and a descriptive message (e.g., "Stream ended without completion ([DONE] missing)"), return false, and only log PASS and return true when textChunks has data and doneReceived is true.examples/client-sdks/vercel-ai-sdk.ts-327-345 (1)
327-345:⚠️ Potential issue | 🟡 MinorUse the package that this PR actually publishes in these snippets.
Both Next.js examples import
createNeuroLinkProviderfrom@neurolink/ai-sdk, but the adapter added in this PR is exposed from@juspay/neurolink/client. Copy-pasting either template currently produces a module resolution failure.Also applies to: 351-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/client-sdks/vercel-ai-sdk.ts` around lines 327 - 345, The example imports the NeuroLink provider from the wrong package; update the import so createNeuroLinkProvider is imported from the package published by this PR (replace imports of createNeuroLinkProvider from '@neurolink/ai-sdk' with the published package, e.g., '@juspay/neurolink/client') wherever it appears (symbols: createNeuroLinkProvider, neurolink) in the Next.js snippets that also call streamText and return StreamingTextResponse so the runtime can resolve the module.src/lib/client/sseClient.ts-514-548 (1)
514-548:⚠️ Potential issue | 🟡 MinorReconnect attempts counter is never reset on successful reconnection.
When
attemptReconnectsuccessfully reconnects by callingthis.stream(), thereconnectAttemptscounter is not reset. If the connection drops again later, it will continue from the previous count and may hitmaxReconnectAttemptsprematurely.🔧 Proposed fix
Add a reset in
stream()after successful connection:this.setState("connected"); this.eventHandlers.onOpen?.(); logger.debug(`[NeuroLinkSSE] Connected to ${url}`); + this.reconnectAttempts = 0; // Reset on successful connection await this.processStream(response, callbacks);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/sseClient.ts` around lines 514 - 548, The reconnectAttempts counter is never reset, so after a successful reconnect the next drop will start from the previous attempt count; update the stream() success path to reset this.reconnectAttempts = 0 (or reset where the connection/open is confirmed) so attemptReconnect() starts fresh on subsequent drops; specifically modify the stream() implementation (the code that runs after a successful connection/handshake/open) to set reconnectAttempts to 0, leaving attemptReconnect, reconnectDelay calculation, and the maxReconnectAttempts checks unchanged.src/lib/client/auth.ts-403-414 (1)
403-414:⚠️ Potential issue | 🟡 MinorToken refresh result is not persisted back to config.
When
refreshToken()succeeds at line 406, the new token is used for the current request butconfig.tokenandconfig.tokenExpiresAtare not updated. Subsequent requests will still see the old (possibly expired) values and trigger refresh again.🔧 Proposed fix
// Handle dynamic token refresh if (config.refreshToken && config.tokenExpiresAt) { const bufferMs = config.refreshBufferMs ?? 60000; if (Date.now() > config.tokenExpiresAt - bufferMs) { const newToken = await config.refreshToken(); + // Update config so subsequent requests use the new token + config.token = newToken; + config.tokenExpiresAt = Date.now() + 3600000; // Or extract from refresh response const headerName = config.headerName ?? "Authorization"; request.headers[headerName] = `Bearer ${newToken}`; } }Note: This requires
configto be mutable or the middleware to maintain internal state. Consider using a token manager pattern similar toOAuth2TokenManagerfor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/auth.ts` around lines 403 - 414, The middleware currently calls config.refreshToken() and uses the returned newToken for the outgoing request but does not persist it back to config, so future requests still use the old token; modify the middleware (the block that checks config.refreshToken, config.tokenExpiresAt, refreshBufferMs and sets request.headers[headerName]) to persist the refresh result: after calling config.refreshToken(), detect whether the result is a string or an object (e.g. { token, expiresAt } or { token, tokenExpiresAt }), set config.token = token (or the string), set config.tokenExpiresAt = expiresAt (or Date.now() + (config.tokenTTLMs ?? 3600000) if expiresAt is not provided), then update request.headers[headerName] = `Bearer ${token}`; this ensures config.token and config.tokenExpiresAt are updated for subsequent requests.src/lib/client/streamingClient.ts-970-976 (1)
970-976:⚠️ Potential issue | 🟡 MinorSSE transport
getState()returns hardcoded "connected".For the SSE transport path,
getState()always returns"connected"regardless of actual connection state. This is inconsistent with the WebSocket transport which returns actual state.🔧 Proposed fix
Track state in the SSE transport or return a more accurate value:
+ let sseState: WebSocketState = "disconnected"; // SSE transport return { - connect: () => Promise.resolve(), - disconnect: () => {}, + connect: () => { sseState = "connected"; return Promise.resolve(); }, + disconnect: () => { sseState = "disconnected"; }, // ... stream implementation ... - getState: () => "connected" as WebSocketState, + getState: () => sseState, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/streamingClient.ts` around lines 970 - 976, The SSE transport currently returns a hardcoded "connected" from getState(), so change getState() on the SSE client to reflect the real EventSource state instead of a constant: either read EventSource.readyState and map its numeric values to your WebSocketState union or maintain an internal state variable updated on the EventSource handlers (onopen -> "connected"/"open", onerror/close -> "closed"/"disconnected", and initial -> "connecting"), then return that value from getState(); update references to send, on, off, getState in the SSE transport implementation accordingly.src/lib/client/httpClient.ts-956-1020 (1)
956-1020:⚠️ Potential issue | 🟡 MinorWebSocket auto-reconnect may cause infinite loop if connection keeps failing.
The
onclosehandler schedules reconnection viasetTimeoutwhenautoReconnectis true andwsState === "disconnected". However, there's no retry limit, so a permanently unavailable server will cause infinite reconnection attempts.🔧 Proposed fix
Add reconnection attempt tracking similar to
WebSocketStreamingClient:private wsReconnectAttempts = 0; + private maxWsReconnectAttempts = 10; // In connectWebSocket: this.wsConnection.onopen = () => { this.wsState = "connected"; + this.wsReconnectAttempts = 0; // Reset on success // ... }; this.wsConnection.onclose = () => { // ... - if (options?.autoReconnect) { + if (options?.autoReconnect && this.wsReconnectAttempts < this.maxWsReconnectAttempts) { + this.wsReconnectAttempts++; const interval = options.reconnectInterval ?? 5000; setTimeout(() => { if (this.wsState === "disconnected") { this.connectWebSocket(options); } }, interval); } };🤖 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 956 - 1020, The onclose handler in connectWebSocket can cause infinite reconnection attempts; add a retry counter (e.g., this.wsReconnectAttempts) and a max retry limit (e.g., options.maxReconnectAttempts or this.config.maxReconnectAttempts) checked before scheduling setTimeout, incrementing the counter on each scheduled reconnect and resetting it to 0 in onopen; additionally, implement backoff by using either options.reconnectInterval as a base and multiplying by the attempt count (or cap at a max interval) and ensure wsState/wsConnection logic uses these symbols (connectWebSocket, wsState, wsConnection, onopen, onclose, options.autoReconnect) to locate and update the code.
🧹 Nitpick comments (7)
examples/client-sdks/react-chat-app.tsx (1)
124-144: Stabilize the demo session ID.
sessionId: \session-${Date.now()}`is regenerated on every render. Even ifuseChat()snapshots it today, this teaches a brittle pattern and will fragment conversations as soon as the hook readsoptions.sessionId` reactively.Possible fix
function ChatTab() { const [selectedImages, setSelectedImages] = useState<string[]>([]); const fileInputRef = useRef<HTMLInputElement>(null); + const sessionIdRef = useRef(`session-${Date.now()}`); @@ } = useChat({ agentId: "general-assistant", - sessionId: `session-${Date.now()}`, + sessionId: sessionIdRef.current, systemPrompt: "You are a helpful assistant. Be concise and friendly.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/client-sdks/react-chat-app.tsx` around lines 124 - 144, The sessionId passed inline to useChat is recreated on every render (sessionId: `session-${Date.now()}`) which can fragment conversations; inside ChatTab create a stable session identifier once (for example with useRef or useMemo) and pass that stable value to useChat (replace the inline template with stableSessionIdRef.current or stableSessionId). Keep the generation logic (e.g., `session-${Date.now()}`) but ensure it runs only once when the component mounts so useChat receives an immutable sessionId across renders.src/lib/client/index.ts (2)
239-245:SSEConnectionStateandSSEStateare both exported.Both types represent SSE connection states but have different values:
SSEConnectionState(from streamingClient.ts): includes"reconnecting"SSEState(from sseClient.ts): includes"error"insteadConsider documenting the difference or unifying them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/index.ts` around lines 239 - 245, There are two overlapping SSE enums/types (SSEConnectionState from streamingClient.js and SSEState from sseClient.ts) with inconsistent literal values ("reconnecting" vs "error"); pick a single canonical type (preferably SSEConnectionState) and unify usages: update sseClient.ts to use and export the canonical SSEConnectionState (or re-export SSEConnectionState from streamingClient.js as SSEState if backward compatibility is required), remove the duplicate definition, and ensure all references and the public export block export only the unified type so the project has one authoritative SSE state type and consistent literal values.
140-152: Potential naming conflict:NeuroLinkProvideris exported from both reactHooks and aiSdkAdapter.Line 141 exports
NeuroLinkProviderfromreactHooks.js(React context provider), while line 182 exportsNeuroLinkProvider as NeuroLinkAIProviderfromaiSdkAdapter.js. The aliasing avoids a direct conflict, but consumers may be confused about whichNeuroLinkProviderto use.Consider renaming the React provider to
NeuroLinkReactProviderorNeuroLinkContextProviderto make the distinction clearer:export { - NeuroLinkProvider, + NeuroLinkProvider as NeuroLinkReactProvider, useNeuroLinkClient, // ... } from "./reactHooks.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/index.ts` around lines 140 - 152, The exported React context provider NeuroLinkProvider from reactHooks.js conflicts conceptually with NeuroLinkProvider (aliased as NeuroLinkAIProvider) from aiSdkAdapter.js; rename the React export and its type to a clearer name (e.g., NeuroLinkReactProvider or NeuroLinkContextProvider) by updating the export statement in src/lib/client/index.ts to export the provider under the new identifier as well as renaming the corresponding type NeuroLinkProviderProps (e.g., NeuroLinkReactProviderProps) in reactHooks.js and adjusting any imports/usages of NeuroLinkProvider, NeuroLinkProviderProps, and the export lines referencing reactHooks.js so consumers clearly distinguish the React context provider from the AI SDK provider.src/lib/client/streamingClient.ts (1)
343-351: Linear backoff may cause rapid reconnection attempts.The reconnect delay uses linear scaling (
reconnectDelay * reconnectAttempts) rather than exponential backoff. For 5 attempts with 1000ms base delay, the delays are 1s, 2s, 3s, 4s, 5s. Consider exponential backoff with jitter for better server protection during outages.💡 Exponential backoff suggestion
- const delayMs = - (this.options.reconnectDelay ?? 1000) * this.reconnectAttempts; + const baseDelay = this.options.reconnectDelay ?? 1000; + const delayMs = Math.min( + baseDelay * Math.pow(2, this.reconnectAttempts - 1), + 30000 // max 30s + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/streamingClient.ts` around lines 343 - 351, The current reconnection logic in SSEClient uses linear backoff: delayMs = (this.options.reconnectDelay ?? 1000) * this.reconnectAttempts; change this to use exponential backoff with jitter to avoid rapid retries: compute a baseDelay (this.options.reconnectDelay ?? 1000), then set delayMs = min(maxDelay, baseDelay * 2^(this.reconnectAttempts - 1)), and apply randomized jitter (e.g., multiply by a random factor between 0.5 and 1.5) before calling sleep(delayMs); update the debug log around logger.debug to show the computed exponential delay and attempt number and ensure reconnectAttempts increments/limits remain respected when calling this.connect(requestOptions).src/lib/client/sseClient.ts (1)
27-27: Inconsistent SSE state types across modules.
SSEStatehere differs fromSSEConnectionStateinstreamingClient.ts:
SSEState:"connecting" | "connected" | "disconnected" | "error"SSEConnectionState:"connecting" | "connected" | "disconnected" | "reconnecting" | "error"Both are exported from
src/lib/client/index.ts, which may confuse consumers. Consider unifying these types or using distinct, descriptive names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/sseClient.ts` at line 27, SSE state types are inconsistent: SSEState and SSEConnectionState differ (SSEState lacks "reconnecting") which confuses consumers; unify them by choosing a single canonical type name (e.g., SSEConnectionState) and updating all exports/usages to use that type, or rename one to a distinct descriptive name if they must differ; update the exported symbol(s) and replace references to SSEState and SSEConnectionState across the codebase (including imports/exports) so both type definitions no longer conflict and include the full union ("connecting" | "connected" | "disconnected" | "reconnecting" | "error") if reconnection is needed.src/lib/client/interceptors.ts (1)
598-620: Cache eviction is FIFO, not LRU.The
SimpleCacheevicts the "oldest" entry usingthis.cache.keys().next().value, which gives the first-inserted key (FIFO) due to Map iteration order. This is not true LRU (least recently used) since accessed entries aren't moved to the end.💡 For true LRU behavior
get(key: string): T | undefined { const entry = this.cache.get(key); if (!entry) { return undefined; } if (Date.now() > entry.expires) { this.cache.delete(key); return undefined; } + // Move to end for LRU behavior + this.cache.delete(key); + this.cache.set(key, entry); return entry.value; }This is acceptable for a simple cache but worth documenting if LRU is expected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/interceptors.ts` around lines 598 - 620, The current eviction in SimpleCache uses Map insertion order (this.cache.keys().next().value) which yields FIFO, not LRU; update the cache to behave LRU by ensuring accesses bump entries to the end of the Map: in the get method (and in set when updating an existing key) remove and re-insert the key so it becomes most-recently-used, keep eviction logic in set to delete the first Map key when size >= maxSize, and continue using this.cache, set, delete, clear and maxSize as the referenced symbols when making these changes.src/lib/client/aiSdkAdapter.ts (1)
297-317: Provider inference has overlapping patterns that may cause incorrect routing.Both
google-aiandvertexpatterns include/^gemini-/and/^palm-/. A model ID likegemini-prowill matchgoogle-aifirst due to object iteration order, which may not be the intended provider for all users.Consider:
- Documenting that
google-aitakes precedence forgemini-*models- Using more specific patterns for Vertex (e.g., require project-qualified model IDs)
- Requiring explicit
provideroption for ambiguous model IDs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/client/aiSdkAdapter.ts` around lines 297 - 317, The inferProvider function uses overlapping regexes in providerPatterns causing ambiguous matches (e.g., gemini-* matches both "google-ai" and "vertex"); update providerPatterns in inferProvider to make Vertex patterns more specific (e.g., require project-qualified IDs like /^projects\/[^\/]+\/locations\/[^\/]+\/models\/(gemini-|palm-)/ and similar for codechat), and add an ambiguity check: if a modelId matches more than one provider, prefer an explicit provider option (check this.options.provider or this.defaultProvider) or throw/log an explicit error asking callers to pass provider; reference inferProvider, providerPatterns, and this.defaultProvider when making changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/client-sdks/node-backend.ts`:
- Around line 281-317: The stream call creates an AbortController that is never
aborted and the streamAgent branch omits any signal, so if the HTTP client
disconnects the upstream provider keeps running; fix by creating a single
AbortController before calling neurolinkClient.stream (and before calling
streamAgent), pass controller.signal into both call sites, and attach a listener
to the request/response close event (e.g., req.on('close') or res.on('close'))
to call controller.abort() so the onDone/onError handlers and upstream are
cancelled when the SSE client disconnects.
In `@examples/client-sdks/react-chat-app.tsx`:
- Around line 796-802: The tool cards rendered from tools currently use a
clickable div (class "tool-card") with onClick only; make selection
keyboard-accessible by converting the div into a semantic interactive element or
adding accessibility props: replace or update the element that uses
key={tool.name} / className={`tool-card ${selectedTool === tool.name ?
"selected" : ""}`} / onClick={() => setSelectedTool(tool.name)} so it is
focusable (tabIndex=0 or use a <button>), exposes role="button" if not a button,
and handles keyboard activation by calling setSelectedTool(tool.name) on Enter
and Space in an onKeyDown handler; also add aria-pressed or aria-selected to
reflect selectedTool for screen readers and ensure visible focus styles.
- Around line 45-48: The environment variables in the client config (the
baseUrl, apiKey, and debug flags) use CRA's process.env.REACT_APP_* pattern
which won't exist under Vite; update the config where baseUrl, apiKey, and debug
are set (the object containing baseUrl, apiKey, debug) to read from
import.meta.env using VITE-prefixed names (e.g., VITE_NEUROLINK_BASE_URL and
VITE_NEUROLINK_API_KEY) and use import.meta.env.DEV or import.meta.env.MODE ===
'development' for the debug check; also ensure your .env uses the VITE_ prefixes
or alternatively inject the values from a backend endpoint if you want
bundler-agnostic config.
In `@package.json`:
- Around line 154-157: The package.json currently exposes a "./client" entry
that includes React code (see src/lib/client/reactHooks.tsx) but React is only a
devDependency; add "react" (and optionally "react-dom" if used) to
package.json's peerDependencies with a compatible semver range (e.g. ^17 || ^18)
so consumers must provide React, and update peerDependenciesMeta if you need to
mark it optional; ensure you remove it from only devDependencies if you want
peer-only.
In `@src/lib/client/aiSdkAdapter.ts`:
- Around line 145-204: doStream() currently buffers all text in createStream()
by awaiting self.client.stream() and then yielding a single text-delta; change
it to produce incremental yields as chunks arrive by turning the client.stream
call into a background/async callback producer that pushes events into a queue
consumed by the async generator. Specifically, in createStream() (inside
doStream()) start self.client.stream(...) without awaiting it and use its onText
to push {type: "text-delta", textDelta} entries, onDone to push a finish event
(use this.mapFinishReason for finishReason and include usage), and onError to
record an error; have the generator await a resolver when the queue is empty and
yield items as they arrive, finally draining remaining items and yielding a
finish with finishReason "error" if an error occurred so the returned
LanguageModelStreamResponse.stream yields incremental updates instead of a
single buffered chunk.
In `@src/lib/client/interceptors.ts`:
- Around line 720-748: createTimeoutInterceptor creates an AbortController and
stores its signal on request.context.timeoutSignal but downstream code (e.g.,
httpClient.request) does not use that signal so requests aren't actually
aborted; either update httpClient.request to respect and merge
request.context.timeoutSignal with its own AbortController (so the fetch call
listens to the interceptor signal), or change createTimeoutInterceptor to
attach/merge its controller.signal into the request init used by downstream
(e.g., set/merge request.init.signal) and ensure the fetch uses that signal;
refer to createTimeoutInterceptor, request.context.timeoutSignal, and
httpClient.request when making the change.
In `@src/lib/client/reactHooks.tsx`:
- Around line 243-247: The outbound request body is being built from the
closure-captured messages array inside append(), causing stale assistant turns
to be sent when reload() mutates state; update append() (and the other
occurrence around the reload flow) to read the latest messages from the current
state (e.g., messagesRef.current or by calling the state getter) instead of
using the captured messages variable so JSON.stringify([...messages,
userMessage].map(...)) always serializes the up-to-date transcript before
sending.
- Around line 180-195: The useChat hook is bypassing the configured
NeuroLinkClient by calling raw fetch() to "/api/chat"; replace the direct fetch
calls in useChat (and the similar raw-fetch transport hooks referenced around
the other transport functions) with the client returned from
useNeuroLinkClient() so requests go through the provider's base URL, auth,
retry/interceptor chain, and typed error handling; specifically, call
useNeuroLinkClient() inside useChat, use its request/post/fetch wrapper (or
client.request) instead of window.fetch, pass the same body/headers/credentials,
and propagate errors via the client's typed error handling while keeping
existing callbacks (onResponse/onFinish/onError/onToolCall) and generateId
behavior.
- Around line 69-77: NeuroLinkProviderProps is a reusable public type defined
locally; move it into the central types module by creating a new/exported
declaration in src/lib/types (e.g., export type NeuroLinkProviderProps = {
config: ClientConfig; children: ReactNode; }), remove the local declaration from
reactHooks.tsx, and update all references/imports of NeuroLinkProviderProps
(including the NeuroLinkProvider component and any tests) to import it from the
new types module; ensure the exported name matches exactly and that ClientConfig
and ReactNode are imported or re-exported as needed.
In `@src/lib/client/wsClient.ts`:
- Around line 291-312: The ws.onopen handler currently only sends auth and
flushes queued messages, so existing channel listeners in the subscriptions map
never resend subscribe frames after a reconnect; modify the onopen block (in
ws.onopen within wsClient.ts) to iterate this.subscriptions (the map that holds
channel -> callbacks) and send a subscribe frame for each channel (same shape
used by your subscribe logic) after clearing pendingAuth and before/alongside
flushMessageQueue, ensuring each subscription is replayed to the server on
reconnect and does not rely on callers to re-subscribe manually; keep calling
this.eventHandlers.onOpen(), this.startHeartbeat(), and this.flushMessageQueue()
as before.
- Around line 208-218: The disconnect() implementation wrongly flips
config.autoReconnect and fails to clear any pending reconnect timer; change it
so disconnect() only performs a manual shutdown (call stopHeartbeat(), close
this.ws, set this.ws = null, setState("disconnected")) and set a new boolean
flag like this.manualDisconnect = true instead of mutating config.autoReconnect;
also add a tracked reconnectTimer property (e.g., this.reconnectTimer) that the
reconnect scheduling code (the place that calls setTimeout for reconnection)
assigns and returns, and ensure disconnect() clears that timer
(clearTimeout(this.reconnectTimer)) so no scheduled reconnect fires after manual
disconnect; finally, ensure connect() clears this.manualDisconnect when invoked
so a subsequent manual connect can re-enable reconnection behavior.
- Around line 23-70: The WebSocket public types (WebSocketState,
WebSocketConfig, WebSocketMessage, WebSocketEventHandlers) were defined locally
in wsClient.ts and diverge from the canonical client types used by
httpClient.ts; move these type definitions into the central
src/lib/types/clientTypes.ts (or update that file) so there is a single source
of truth, update their literal unions/shape to match the canonical definitions
used by httpClient.ts (e.g., reconcile "error" vs "disconnecting" |
"reconnecting"), then remove the local type declarations from wsClient.ts and
import the types from clientTypes.ts wherever wsClient.ts and other modules
(like httpClient.ts) consume them. Ensure exports/imports are updated so
consumers use the shared types.
- Around line 186-199: The WebSocket constructor usage in wsClient.ts is
incorrect: the native/browser WebSocket doesn't accept a headers option and the
current cast around new WebSocket(...) hides the bug, so Node clients won't send
auth headers; update the implementation to explicitly use the ws package
WebSocket in Node (import WebSocket from 'ws') and construct the socket with the
third-argument options object containing headers when isNode and authHeaders is
non-empty (e.g. new WebSocket(url.toString(), undefined, { headers: authHeaders
})), or alternatively unify behavior by always sending credentials as the first
post-open message (reuse the existing post-open auth message path) so both
browser and Node follow the same auth flow; modify the code paths around isNode,
authHeaders and this.ws accordingly.
In `@src/lib/index.ts`:
- Around line 47-128: Remove all React-related exports (NeuroLinkProvider,
useNeuroLinkClient, useChat, useAgent, useWorkflow, useVoice, useStream,
useTools and any related type exports) from the root barrel in src/lib/index.ts
so they are only provided via the client subpath; update the export list in the
file to exclude these symbols and ensure only non-React APIs (e.g.,
NeuroLinkClient, createClient, NeuroLinkApiError, AI SDK Adapter, interceptors,
streaming client, authentication, and error exports) remain in the root export
block, leaving React hooks exported solely from ./client/index.js (the new
`@juspay/neurolink/client` entry).
In `@src/lib/server/adapters/honoAdapter.ts`:
- Around line 430-444: The streamed error frames sent in the isErrorResponse
branch (using stream.writeSSE with the errorResult object) omit the required
ApiError.status field, causing clients to see undefined status; update the
payload produced in the isErrorResponse branches (the block handling result &&
isErrorResponse(result) and the similar block around lines 457-465) to include
status: errorResult.error?.status ?? a sensible default (e.g., 500) so
StreamEvent.error meets the ApiError shape; ensure you reference the same
errorResult and StreamEvent.error structure when building the JSON passed to
stream.writeSSE.
- Around line 445-449: The SSE frame for single-result paths uses
JSON.stringify(result) which lacks the StreamEvent shape and a required type, so
handleEvent() ignores it; update the else branch around stream.writeSSE to
normalize the result into a StreamEvent object (include the same "type" field
used by the stream parser and any other fields expected by handleEvent) before
stringifying and sending; reference the existing StreamEvent shape/creator used
elsewhere in honoAdapter.ts (and ensure compatibility with handleEvent and
stream.writeSSE) so one-shot responses are parsed and accumulated correctly.
In `@src/lib/types/clientTypes.ts`:
- Around line 181-198: StreamCallbacks now includes onMetadata, onAudio, and
onThinking but the SSE client's event dispatch logic only handles
text/tool-call/tool-result/done/error so those callbacks never run; update the
SSE client's event handler (the function that parses incoming SSE events and
currently dispatches text, tool-call, tool-result, done, error) to detect the
new event types (e.g., "metadata", "audio", "thinking"), parse their payloads
into JsonObject / {data, format} / string as appropriate, and invoke onMetadata,
onAudio, and onThinking when present (preserving existing error parsing for
ApiError and onDone for StreamResult). Ensure you handle malformed payloads
safely and keep typings consistent with StreamCallbacks.
- Around line 998-1052: The AuthConfig, OAuth2Config, and TokenRefreshResult
type declarations are duplicated here; remove these local declarations and
instead use the canonical auth types module's definitions: delete the
AuthConfig, OAuth2Config, and TokenRefreshResult blocks and replace them with
imports (and re-exports if needed) pointing to the single source of truth so the
client entry references the same types (keep references to AuthConfig,
OAuth2Config, and TokenRefreshResult in the file but have them
imported/re-exported from the canonical auth types module).
In `@test/continuous-test-suite-client.ts`:
- Around line 134-135: SERVER_URL is computed too early using
TEST_CONFIG.serverPort (const SERVER_URL =
`http://localhost:${TEST_CONFIG.serverPort}`) before CLI parsing mutates
TEST_CONFIG.serverPort; update the code to compute SERVER_URL after CLI args are
parsed (or replace the const with a function/getter that reads
TEST_CONFIG.serverPort at use-time) and ensure any derived URLs (SSE/client URL
constructions in the same file) use this late-computed value so the --port
override is respected.
---
Outside diff comments:
In `@package.json`:
- Around line 64-79: The default CI test gate doesn't run the new client suite;
update the umbrella test invocation so the client tests run under the default
gate—either (A) compose continuous-test-suite-client.ts into the master umbrella
test runner (continuous-test-suite.ts) by importing/executing the client suite
from within the existing runner, or (B) change the test aggregation script (what
"pnpm run test" triggers / the "test:ci" / "test" script) to invoke both
continuous-test-suite.ts and continuous-test-suite-client.ts (or run the named
scripts "test:client" alongside the existing suite) so that test:client is
executed by the CI gate. Ensure you modify the script entries referencing
test:ci / test / pnpm run test accordingly and keep naming consistent with the
existing script keys (test:client, test:ci, test).
---
Minor comments:
In `@examples/client-sdks/vercel-ai-sdk.ts`:
- Around line 327-345: The example imports the NeuroLink provider from the wrong
package; update the import so createNeuroLinkProvider is imported from the
package published by this PR (replace imports of createNeuroLinkProvider from
'@neurolink/ai-sdk' with the published package, e.g.,
'@juspay/neurolink/client') wherever it appears (symbols:
createNeuroLinkProvider, neurolink) in the Next.js snippets that also call
streamText and return StreamingTextResponse so the runtime can resolve the
module.
In `@src/lib/client/auth.ts`:
- Around line 403-414: The middleware currently calls config.refreshToken() and
uses the returned newToken for the outgoing request but does not persist it back
to config, so future requests still use the old token; modify the middleware
(the block that checks config.refreshToken, config.tokenExpiresAt,
refreshBufferMs and sets request.headers[headerName]) to persist the refresh
result: after calling config.refreshToken(), detect whether the result is a
string or an object (e.g. { token, expiresAt } or { token, tokenExpiresAt }),
set config.token = token (or the string), set config.tokenExpiresAt = expiresAt
(or Date.now() + (config.tokenTTLMs ?? 3600000) if expiresAt is not provided),
then update request.headers[headerName] = `Bearer ${token}`; this ensures
config.token and config.tokenExpiresAt are updated for subsequent requests.
In `@src/lib/client/httpClient.ts`:
- Around line 956-1020: The onclose handler in connectWebSocket can cause
infinite reconnection attempts; add a retry counter (e.g.,
this.wsReconnectAttempts) and a max retry limit (e.g.,
options.maxReconnectAttempts or this.config.maxReconnectAttempts) checked before
scheduling setTimeout, incrementing the counter on each scheduled reconnect and
resetting it to 0 in onopen; additionally, implement backoff by using either
options.reconnectInterval as a base and multiplying by the attempt count (or cap
at a max interval) and ensure wsState/wsConnection logic uses these symbols
(connectWebSocket, wsState, wsConnection, onopen, onclose,
options.autoReconnect) to locate and update the code.
In `@src/lib/client/sseClient.ts`:
- Around line 514-548: The reconnectAttempts counter is never reset, so after a
successful reconnect the next drop will start from the previous attempt count;
update the stream() success path to reset this.reconnectAttempts = 0 (or reset
where the connection/open is confirmed) so attemptReconnect() starts fresh on
subsequent drops; specifically modify the stream() implementation (the code that
runs after a successful connection/handshake/open) to set reconnectAttempts to
0, leaving attemptReconnect, reconnectDelay calculation, and the
maxReconnectAttempts checks unchanged.
In `@src/lib/client/streamingClient.ts`:
- Around line 970-976: The SSE transport currently returns a hardcoded
"connected" from getState(), so change getState() on the SSE client to reflect
the real EventSource state instead of a constant: either read
EventSource.readyState and map its numeric values to your WebSocketState union
or maintain an internal state variable updated on the EventSource handlers
(onopen -> "connected"/"open", onerror/close -> "closed"/"disconnected", and
initial -> "connecting"), then return that value from getState(); update
references to send, on, off, getState in the SSE transport implementation
accordingly.
In `@src/lib/server/adapters/honoAdapter.ts`:
- Around line 512-517: The code in honoAdapter.ts currently emits an unsupported
event object with type "data" for unknown chunk shapes; instead, stop emitting
that unsupported "data" event (which is not part of StreamEventType) — change
the fallback to return undefined/null (or throw) so the caller drops the chunk,
and add a debug/processLogger.warn indicating an unexpected chunk shape; ensure
the function's return type allows undefined and that the caller filters out
falsy results, and reference StreamEventType and the SSE client handlers in
src/lib/client/sseClient.ts to confirm no "data" handling is required.
In `@test/continuous-test-suite-client.ts`:
- Around line 289-304: The current post-stream check in the test (variables
result, content, textChunks, doneReceived, and function logTest) marks the test
PASS as long as any text arrived, but it must also ensure the SSE completion
event was received; update the logic so that if doneReceived is false you call
logTest with a FAIL and a descriptive message (e.g., "Stream ended without
completion ([DONE] missing)"), return false, and only log PASS and return true
when textChunks has data and doneReceived is true.
---
Nitpick comments:
In `@examples/client-sdks/react-chat-app.tsx`:
- Around line 124-144: The sessionId passed inline to useChat is recreated on
every render (sessionId: `session-${Date.now()}`) which can fragment
conversations; inside ChatTab create a stable session identifier once (for
example with useRef or useMemo) and pass that stable value to useChat (replace
the inline template with stableSessionIdRef.current or stableSessionId). Keep
the generation logic (e.g., `session-${Date.now()}`) but ensure it runs only
once when the component mounts so useChat receives an immutable sessionId across
renders.
In `@src/lib/client/aiSdkAdapter.ts`:
- Around line 297-317: The inferProvider function uses overlapping regexes in
providerPatterns causing ambiguous matches (e.g., gemini-* matches both
"google-ai" and "vertex"); update providerPatterns in inferProvider to make
Vertex patterns more specific (e.g., require project-qualified IDs like
/^projects\/[^\/]+\/locations\/[^\/]+\/models\/(gemini-|palm-)/ and similar for
codechat), and add an ambiguity check: if a modelId matches more than one
provider, prefer an explicit provider option (check this.options.provider or
this.defaultProvider) or throw/log an explicit error asking callers to pass
provider; reference inferProvider, providerPatterns, and this.defaultProvider
when making changes.
In `@src/lib/client/index.ts`:
- Around line 239-245: There are two overlapping SSE enums/types
(SSEConnectionState from streamingClient.js and SSEState from sseClient.ts) with
inconsistent literal values ("reconnecting" vs "error"); pick a single canonical
type (preferably SSEConnectionState) and unify usages: update sseClient.ts to
use and export the canonical SSEConnectionState (or re-export SSEConnectionState
from streamingClient.js as SSEState if backward compatibility is required),
remove the duplicate definition, and ensure all references and the public export
block export only the unified type so the project has one authoritative SSE
state type and consistent literal values.
- Around line 140-152: The exported React context provider NeuroLinkProvider
from reactHooks.js conflicts conceptually with NeuroLinkProvider (aliased as
NeuroLinkAIProvider) from aiSdkAdapter.js; rename the React export and its type
to a clearer name (e.g., NeuroLinkReactProvider or NeuroLinkContextProvider) by
updating the export statement in src/lib/client/index.ts to export the provider
under the new identifier as well as renaming the corresponding type
NeuroLinkProviderProps (e.g., NeuroLinkReactProviderProps) in reactHooks.js and
adjusting any imports/usages of NeuroLinkProvider, NeuroLinkProviderProps, and
the export lines referencing reactHooks.js so consumers clearly distinguish the
React context provider from the AI SDK provider.
In `@src/lib/client/interceptors.ts`:
- Around line 598-620: The current eviction in SimpleCache uses Map insertion
order (this.cache.keys().next().value) which yields FIFO, not LRU; update the
cache to behave LRU by ensuring accesses bump entries to the end of the Map: in
the get method (and in set when updating an existing key) remove and re-insert
the key so it becomes most-recently-used, keep eviction logic in set to delete
the first Map key when size >= maxSize, and continue using this.cache, set,
delete, clear and maxSize as the referenced symbols when making these changes.
In `@src/lib/client/sseClient.ts`:
- Line 27: SSE state types are inconsistent: SSEState and SSEConnectionState
differ (SSEState lacks "reconnecting") which confuses consumers; unify them by
choosing a single canonical type name (e.g., SSEConnectionState) and updating
all exports/usages to use that type, or rename one to a distinct descriptive
name if they must differ; update the exported symbol(s) and replace references
to SSEState and SSEConnectionState across the codebase (including
imports/exports) so both type definitions no longer conflict and include the
full union ("connecting" | "connected" | "disconnected" | "reconnecting" |
"error") if reconnection is needed.
In `@src/lib/client/streamingClient.ts`:
- Around line 343-351: The current reconnection logic in SSEClient uses linear
backoff: delayMs = (this.options.reconnectDelay ?? 1000) *
this.reconnectAttempts; change this to use exponential backoff with jitter to
avoid rapid retries: compute a baseDelay (this.options.reconnectDelay ?? 1000),
then set delayMs = min(maxDelay, baseDelay * 2^(this.reconnectAttempts - 1)),
and apply randomized jitter (e.g., multiply by a random factor between 0.5 and
1.5) before calling sleep(delayMs); update the debug log around logger.debug to
show the computed exponential delay and attempt number and ensure
reconnectAttempts increments/limits remain respected when calling
this.connect(requestOptions).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6bebc11-82bf-4abe-8faa-eadd4fc111e5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
docs/features/client-sdk.mddocs/features/index.mdexamples/client-sdks/node-backend.tsexamples/client-sdks/react-chat-app.tsxexamples/client-sdks/vercel-ai-sdk.tsexamples/client-sdks/websocket-realtime.tspackage.jsonsrc/lib/client/aiSdkAdapter.tssrc/lib/client/auth.tssrc/lib/client/errors.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/index.tssrc/lib/server/adapters/honoAdapter.tssrc/lib/types/clientTypes.tssrc/lib/types/index.tstest/continuous-test-suite-client.tstsconfig.cli.jsontsconfig.json
Comment on lines
+281
to
+317
| await neurolinkClient.stream( | ||
| options, | ||
| { | ||
| onText: (text) => { | ||
| res.write( | ||
| `data: ${JSON.stringify({ type: "text", content: text })}\n\n`, | ||
| ); | ||
| }, | ||
| onToolCall: (toolCall) => { | ||
| res.write( | ||
| `data: ${JSON.stringify({ type: "tool-call", toolCall })}\n\n`, | ||
| ); | ||
| }, | ||
| onToolResult: (toolResult) => { | ||
| res.write( | ||
| `data: ${JSON.stringify({ type: "tool-result", toolResult })}\n\n`, | ||
| ); | ||
| }, | ||
| onMetadata: (metadata) => { | ||
| res.write( | ||
| `data: ${JSON.stringify({ type: "metadata", metadata })}\n\n`, | ||
| ); | ||
| }, | ||
| onDone: (result) => { | ||
| res.write(`data: ${JSON.stringify({ type: "done", result })}\n\n`); | ||
| res.write("data: [DONE]\n\n"); | ||
| res.end(); | ||
| }, | ||
| onError: (error) => { | ||
| res.write(`data: ${JSON.stringify({ type: "error", error })}\n\n`); | ||
| res.end(); | ||
| }, | ||
| }, | ||
| { | ||
| signal: req.socket.destroyed ? new AbortController().signal : undefined, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Abort upstream streaming when the HTTP client disconnects.
Line 315 creates a fresh AbortController but never aborts it, and the streamAgent() branch does not pass any signal at all. If the browser closes the SSE connection, the provider stream keeps running in the background and continues consuming resources.
Suggested fix
app.post("/api/stream", async (req: Request, res: Response) => {
try {
+ const abortController = new AbortController();
+ req.on("close", () => abortController.abort());
+
const options: GenerateRequestOptions = {
input: {
text: req.body.input?.text || req.body.prompt,
@@
},
},
- {
- signal: req.socket.destroyed ? new AbortController().signal : undefined,
- },
+ { signal: abortController.signal },
);
} catch (error) {
if (!res.headersSent) {
handleApiError(error, res);
@@
app.post(
"/api/agents/:agentId/execute",
async (req: Request, res: Response) => {
try {
+ const abortController = new AbortController();
+ req.on("close", () => abortController.abort());
+
const options: AgentExecuteOptions = {
agentId: req.params.agentId,
input: req.body.input,
@@
- await neurolinkClient.streamAgent(options, {
- onText: (text) => {
- res.write(
- `data: ${JSON.stringify({ type: "text", content: text })}\n\n`,
- );
- },
- onDone: (result) => {
- res.write(`data: ${JSON.stringify({ type: "done", result })}\n\n`);
- res.write("data: [DONE]\n\n");
- res.end();
- },
- });
+ await neurolinkClient.streamAgent(
+ options,
+ {
+ onText: (text) => {
+ res.write(
+ `data: ${JSON.stringify({ type: "text", content: text })}\n\n`,
+ );
+ },
+ onDone: (result) => {
+ res.write(`data: ${JSON.stringify({ type: "done", result })}\n\n`);
+ res.write("data: [DONE]\n\n");
+ res.end();
+ },
+ },
+ { signal: abortController.signal },
+ );
} else {Also applies to: 357-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/client-sdks/node-backend.ts` around lines 281 - 317, The stream call
creates an AbortController that is never aborted and the streamAgent branch
omits any signal, so if the HTTP client disconnects the upstream provider keeps
running; fix by creating a single AbortController before calling
neurolinkClient.stream (and before calling streamAgent), pass controller.signal
into both call sites, and attach a listener to the request/response close event
(e.g., req.on('close') or res.on('close')) to call controller.abort() so the
onDone/onError handlers and upstream are cancelled when the SSE client
disconnects.
Comment on lines
+45
to
+48
| baseUrl: | ||
| process.env.REACT_APP_NEUROLINK_BASE_URL || "http://localhost:3000", | ||
| apiKey: process.env.REACT_APP_NEUROLINK_API_KEY, | ||
| debug: process.env.NODE_ENV === "development", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For a Vite React application, are browser environment variables exposed via process.env.REACT_APP_, or via import.meta.env.VITE_?
💡 Result:
In a Vite React application, browser environment variables are exposed via import.meta.env.VITE_, not process.env.REACT_APP_. Vite uses import.meta.env to expose environment variables prefixed with VITE_ (by default) to client-side code. Only variables with the VITE_ prefix are exposed to prevent accidental leakage. For example, VITE_API_KEY is accessed as import.meta.env.VITE_API_KEY. This differs from Create React App, which uses process.env.REACT_APP_. Vite does not support process.env in browser code by default; attempting process.env.REACT_APP_ will fail as process is undefined in the browser. The prefix can be customized via envPrefix in vite.config.js, but VITE_ is the standard.
Citations:
- 1: https://v6.vite.dev/guide/env-and-mode
- 2: https://vite.dev/guide/env-and-mode
- 3: https://iere.org/how-to-read-environmental-variable-in-vite-react/
- 4: https://dev.to/stm-akikaze1119/understanding-environment-variables-in-vite-why-vite-prefix-is-important-5c3k
- 5: https://stackoverflow.com/questions/70883903/loading-env-variables-in-react-app-using-vite
- 6: https://stackoverflow.com/questions/70709987/how-to-load-environment-variables-from-env-file-using-vite
- 7: https://vitejs.dev/guide/env-and-mode
🏁 Script executed:
# Check if Vite is configured in the repo
find . -maxdepth 3 -name "vite.config.*" -o -name "package.json" | head -20Repository: juspay/neurolink
Length of output: 221
🏁 Script executed:
# Check CLAUDE.md for environment variable guidance
cat CLAUDE.md | head -100Repository: juspay/neurolink
Length of output: 4498
🏁 Script executed:
# Verify the actual content of the file under review
cat -n examples/client-sdks/react-chat-app.tsx | sed -n '40,55p'Repository: juspay/neurolink
Length of output: 618
🏁 Script executed:
# Look for .env.example or similar configuration files
find . -maxdepth 3 \( -name ".env*" -o -name "*env.example*" \) -type fRepository: juspay/neurolink
Length of output: 175
🏁 Script executed:
# Check the root vite.config.ts
cat -n vite.config.ts | head -50Repository: juspay/neurolink
Length of output: 2201
🏁 Script executed:
# Check .env.example to see documented env vars
cat .env.example | head -50Repository: juspay/neurolink
Length of output: 2368
🏁 Script executed:
# Check if examples directory has vite setup
ls -la examples/ | head -20Repository: juspay/neurolink
Length of output: 1329
🏁 Script executed:
# Check if there's a vite.config for examples
find examples -name "vite.config.*" -o -name "package.json"Repository: juspay/neurolink
Length of output: 191
🏁 Script executed:
# Check what's in examples/client-sdks
ls -la examples/client-sdks/Repository: juspay/neurolink
Length of output: 453
🏁 Script executed:
# Check if react-chat-app has any build setup
find examples -name "package.json" -exec grep -l "react\|vite\|webpack" {} \;Repository: juspay/neurolink
Length of output: 105
🏁 Script executed:
# Check the full react-chat-app.tsx to understand context
wc -l examples/client-sdks/react-chat-app.tsxRepository: juspay/neurolink
Length of output: 104
🏁 Script executed:
# Check example projects to see if any use process.env
rg "process\.env\." examples/ -A 2 -B 2Repository: juspay/neurolink
Length of output: 29581
Replace CRA-style REACT_APP_* env vars with Vite's import.meta.env.VITE_* pattern.
This React example uses process.env.REACT_APP_*, which is from Create React App. The repo uses Vite, which does not expose process in browser code; these variables will be undefined at runtime. Switch to import.meta.env.VITE_* (e.g., VITE_NEUROLINK_BASE_URL) and update the .env configuration to match, or inject config from a backend endpoint to remain bundler-agnostic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/client-sdks/react-chat-app.tsx` around lines 45 - 48, The
environment variables in the client config (the baseUrl, apiKey, and debug
flags) use CRA's process.env.REACT_APP_* pattern which won't exist under Vite;
update the config where baseUrl, apiKey, and debug are set (the object
containing baseUrl, apiKey, debug) to read from import.meta.env using
VITE-prefixed names (e.g., VITE_NEUROLINK_BASE_URL and VITE_NEUROLINK_API_KEY)
and use import.meta.env.DEV or import.meta.env.MODE === 'development' for the
debug check; also ensure your .env uses the VITE_ prefixes or alternatively
inject the values from a backend endpoint if you want bundler-agnostic config.
Comment on lines
+796
to
+802
| <div className="tools-grid"> | ||
| {tools.map((tool) => ( | ||
| <div | ||
| key={tool.name} | ||
| className={`tool-card ${selectedTool === tool.name ? "selected" : ""}`} | ||
| onClick={() => setSelectedTool(tool.name)} | ||
| > |
There was a problem hiding this comment.
Make tool selection keyboard-accessible.
The selectable tool card is a clickable div with no focus or keyboard handling, so keyboard-only users cannot choose a tool to execute from this screen.
Possible fix
<div
key={tool.name}
+ role="button"
+ tabIndex={0}
className={`tool-card ${selectedTool === tool.name ? "selected" : ""}`}
onClick={() => setSelectedTool(tool.name)}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ setSelectedTool(tool.name);
+ }
+ }}
>🧰 Tools
🪛 Biome (2.4.7)
[error] 798-802: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
(lint/a11y/useKeyWithClickEvents)
[error] 798-802: Static Elements should not be interactive.
(lint/a11y/noStaticElementInteractions)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/client-sdks/react-chat-app.tsx` around lines 796 - 802, The tool
cards rendered from tools currently use a clickable div (class "tool-card") with
onClick only; make selection keyboard-accessible by converting the div into a
semantic interactive element or adding accessibility props: replace or update
the element that uses key={tool.name} / className={`tool-card ${selectedTool ===
tool.name ? "selected" : ""}`} / onClick={() => setSelectedTool(tool.name)} so
it is focusable (tabIndex=0 or use a <button>), exposes role="button" if not a
button, and handles keyboard activation by calling setSelectedTool(tool.name) on
Enter and Space in an onKeyDown handler; also add aria-pressed or aria-selected
to reflect selectedTool for screen readers and ensure visible focus styles.
Comment on lines
+154
to
+157
| "./client": { | ||
| "types": "./dist/client/index.d.ts", | ||
| "import": "./dist/client/index.js", | ||
| "default": "./dist/client/index.js" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether the published client sources import React / ReactDOM, and compare that to the manifest.
rg -n --glob 'src/lib/client/**/*.{ts,tsx}' 'from "react"|from "react-dom"'
python - <<'PY'
import json, pathlib
pkg = json.loads(pathlib.Path("package.json").read_text())
print("peerDependencies:", pkg.get("peerDependencies", {}))
print("devDependencies.react:", pkg.get("devDependencies", {}).get("react"))
print("devDependencies.react-dom:", pkg.get("devDependencies", {}).get("react-dom"))
PYRepository: juspay/neurolink
Length of output: 384
Declare React as a peer dependency for the published client entry.
The ./client export includes React hooks code (src/lib/client/reactHooks.tsx imports from "react"), but the package manifest lists React only as a devDependency. react must be declared in peerDependencies to ensure consumers can resolve it correctly and avoid duplicate or missing React instances at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 154 - 157, The package.json currently exposes a
"./client" entry that includes React code (see src/lib/client/reactHooks.tsx)
but React is only a devDependency; add "react" (and optionally "react-dom" if
used) to package.json's peerDependencies with a compatible semver range (e.g.
^17 || ^18) so consumers must provide React, and update peerDependenciesMeta if
you need to mark it optional; ensure you remove it from only devDependencies if
you want peer-only.
Comment on lines
+145
to
+204
| // Create an async generator for the stream | ||
| async function* createStream(): AsyncIterable<{ | ||
| type: "text-delta" | "finish"; | ||
| textDelta?: string; | ||
| finishReason?: string; | ||
| usage?: { promptTokens: number; completionTokens: number }; | ||
| }> { | ||
| let fullText = ""; | ||
|
|
||
| try { | ||
| const result = await self.client.stream( | ||
| { | ||
| input: { text: inputText }, | ||
| provider: self.provider, | ||
| model: self.modelId, | ||
| temperature: temperature ?? self.options.temperature, | ||
| maxTokens: maxTokens ?? self.options.maxTokens, | ||
| systemPrompt, | ||
| }, | ||
| { | ||
| onText: (text) => { | ||
| fullText += text; | ||
| }, | ||
| }, | ||
| { | ||
| signal: abortSignal, | ||
| }, | ||
| ); | ||
|
|
||
| // Yield the accumulated text as a single delta | ||
| if (fullText) { | ||
| yield { | ||
| type: "text-delta" as const, | ||
| textDelta: fullText, | ||
| }; | ||
| } | ||
|
|
||
| // Yield finish event | ||
| yield { | ||
| type: "finish" as const, | ||
| finishReason: self.mapFinishReason(result.finishReason), | ||
| usage: result.usage | ||
| ? { | ||
| promptTokens: result.usage.promptTokens, | ||
| completionTokens: result.usage.completionTokens, | ||
| } | ||
| : undefined, | ||
| }; | ||
| } catch { | ||
| yield { | ||
| type: "finish" as const, | ||
| finishReason: "error", | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| stream: createStream(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
doStream() buffers all content before yielding, defeating streaming purpose.
The doStream() implementation waits for this.client.stream() to complete (accumulating all text via onText callback), then yields a single text-delta with the full content. This defeats the purpose of streaming as consumers won't receive incremental updates.
Per the LanguageModelStreamResponse type in clientTypes.ts, the stream should yield chunks as they arrive. The current implementation is effectively non-streaming.
🔧 Proposed fix using async queue pattern
async doStream(
options: LanguageModelCallOptions,
): Promise<LanguageModelStreamResponse> {
// ... setup code ...
const self = this;
async function* createStream() {
const queue: Array<{ type: "text-delta" | "finish"; textDelta?: string; finishReason?: string; usage?: object }> = [];
let done = false;
let resolver: (() => void) | null = null;
let streamError: Error | null = null;
// Start streaming in background
self.client.stream(
{ /* request options */ },
{
onText: (text) => {
queue.push({ type: "text-delta", textDelta: text });
resolver?.();
},
onDone: (result) => {
queue.push({
type: "finish",
finishReason: self.mapFinishReason(result.finishReason),
usage: result.usage,
});
done = true;
resolver?.();
},
onError: (error) => {
streamError = new Error(error.message);
resolver?.();
},
},
).catch((err) => {
streamError = err;
resolver?.();
});
while (!done && !streamError) {
if (queue.length > 0) {
yield queue.shift()!;
} else {
await new Promise<void>((r) => { resolver = r; });
}
}
// Yield remaining
while (queue.length > 0) {
yield queue.shift()!;
}
if (streamError) {
yield { type: "finish" as const, finishReason: "error" };
}
}
return { stream: createStream() };
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/client/aiSdkAdapter.ts` around lines 145 - 204, doStream() currently
buffers all text in createStream() by awaiting self.client.stream() and then
yielding a single text-delta; change it to produce incremental yields as chunks
arrive by turning the client.stream call into a background/async callback
producer that pushes events into a queue consumed by the async generator.
Specifically, in createStream() (inside doStream()) start
self.client.stream(...) without awaiting it and use its onText to push {type:
"text-delta", textDelta} entries, onDone to push a finish event (use
this.mapFinishReason for finishReason and include usage), and onError to record
an error; have the generator await a resolver when the queue is empty and yield
items as they arrive, finally draining remaining items and yielding a finish
with finishReason "error" if an error occurred so the returned
LanguageModelStreamResponse.stream yields incremental updates instead of a
single buffered chunk.
Comment on lines
+430
to
+444
| } else if (result && isErrorResponse(result)) { | ||
| // Error result, send as error event | ||
| const errorResult = result as { | ||
| error?: { message?: string; code?: string }; | ||
| }; | ||
| await stream.writeSSE({ | ||
| data: JSON.stringify({ | ||
| type: "error", | ||
| error: { | ||
| code: errorResult.error?.code ?? "STREAM_ERROR", | ||
| message: errorResult.error?.message ?? "Stream error", | ||
| }, | ||
| timestamp: Date.now(), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Include status in streamed error frames.
StreamEvent.error is typed as ApiError, where status is required in src/lib/types/clientTypes.ts:97-110. These payloads only send code and message, so client retry/rate-limit branches see error.status === undefined for both handler errors and runtime stream failures.
Possible fix
error: {
code: errorResult.error?.code ?? "STREAM_ERROR",
message: errorResult.error?.message ?? "Stream error",
+ status: result.httpStatus ?? 500,
},
@@
error: {
code: "STREAM_ERROR",
message: error instanceof Error ? error.message : "Stream error",
+ status: 500,
},Also applies to: 457-465
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/server/adapters/honoAdapter.ts` around lines 430 - 444, The streamed
error frames sent in the isErrorResponse branch (using stream.writeSSE with the
errorResult object) omit the required ApiError.status field, causing clients to
see undefined status; update the payload produced in the isErrorResponse
branches (the block handling result && isErrorResponse(result) and the similar
block around lines 457-465) to include status: errorResult.error?.status ?? a
sensible default (e.g., 500) so StreamEvent.error meets the ApiError shape;
ensure you reference the same errorResult and StreamEvent.error structure when
building the JSON passed to stream.writeSSE.
Comment on lines
445
to
449
| } else { | ||
| // Single result, send as complete event | ||
| await stream.writeSSE({ | ||
| data: JSON.stringify(result), | ||
| event: "complete", | ||
| }); |
There was a problem hiding this comment.
Normalize one-shot results before sending SSE.
The SSE client parses every frame as a StreamEvent. Sending raw JSON.stringify(result) here produces objects with no type, so handleEvent() ignores the frame and [DONE] closes the stream with empty accumulated content.
Possible fix
} else {
// Single result, send as complete event
await stream.writeSSE({
- data: JSON.stringify(result),
+ data: JSON.stringify(this.toStreamEvent(result)),
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/server/adapters/honoAdapter.ts` around lines 445 - 449, The SSE frame
for single-result paths uses JSON.stringify(result) which lacks the StreamEvent
shape and a required type, so handleEvent() ignores it; update the else branch
around stream.writeSSE to normalize the result into a StreamEvent object
(include the same "type" field used by the stream parser and any other fields
expected by handleEvent) before stringifying and sending; reference the existing
StreamEvent shape/creator used elsewhere in honoAdapter.ts (and ensure
compatibility with handleEvent and stream.writeSSE) so one-shot responses are
parsed and accumulated correctly.
Comment on lines
+181
to
+198
| export type StreamCallbacks = { | ||
| /** Called for each text chunk */ | ||
| onText?: (text: string) => void; | ||
| /** Called for each tool call */ | ||
| onToolCall?: (toolCall: ToolCall) => void; | ||
| /** Called for each tool result */ | ||
| onToolResult?: (toolResult: ToolResult) => void; | ||
| /** Called on stream error */ | ||
| onError?: (error: ApiError) => void; | ||
| /** Called when stream completes */ | ||
| onDone?: (result: StreamResult) => void; | ||
| /** Called for metadata updates */ | ||
| onMetadata?: (metadata: JsonObject) => void; | ||
| /** Called for audio chunks */ | ||
| onAudio?: (audio: { data: string; format: string }) => void; | ||
| /** Called for thinking/reasoning output */ | ||
| onThinking?: (thinking: string) => void; | ||
| }; |
There was a problem hiding this comment.
Wire the new stream callbacks through the SSE client first.
StreamCallbacks now exposes onMetadata, onAudio, and onThinking, but src/lib/client/sseClient.ts:472-507 only dispatches text/tool-call/tool-result/done/error. On the default SSE path those callbacks never fire. Either add the missing cases there or keep them out of the public callback surface for now.
🤖 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 181 - 198, StreamCallbacks now
includes onMetadata, onAudio, and onThinking but the SSE client's event dispatch
logic only handles text/tool-call/tool-result/done/error so those callbacks
never run; update the SSE client's event handler (the function that parses
incoming SSE events and currently dispatches text, tool-call, tool-result, done,
error) to detect the new event types (e.g., "metadata", "audio", "thinking"),
parse their payloads into JsonObject / {data, format} / string as appropriate,
and invoke onMetadata, onAudio, and onThinking when present (preserving existing
error parsing for ApiError and onDone for StreamResult). Ensure you handle
malformed payloads safely and keep typings consistent with StreamCallbacks.
Comment on lines
+998
to
+1052
| // ============================================================================= | ||
| // Authentication Types | ||
| // ============================================================================= | ||
|
|
||
| /** | ||
| * Authentication configuration options | ||
| */ | ||
| export type AuthConfig = { | ||
| /** API key for header-based authentication */ | ||
| apiKey?: string; | ||
| /** Bearer token for JWT/OAuth authentication */ | ||
| token?: string; | ||
| /** Token refresh function for automatic token renewal */ | ||
| refreshToken?: () => Promise<string>; | ||
| /** Token expiry time in milliseconds */ | ||
| tokenExpiresAt?: number; | ||
| /** Buffer time before expiry to refresh token (default: 60000ms) */ | ||
| refreshBufferMs?: number; | ||
| /** Custom authorization header name (default: "Authorization") */ | ||
| headerName?: string; | ||
| /** Custom API key header name (default: "X-API-Key") */ | ||
| apiKeyHeaderName?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * OAuth2 client credentials configuration | ||
| */ | ||
| export type OAuth2Config = { | ||
| /** Token endpoint URL */ | ||
| tokenUrl: string; | ||
| /** OAuth2 client ID */ | ||
| clientId: string; | ||
| /** OAuth2 client secret */ | ||
| clientSecret: string; | ||
| /** OAuth2 scope (optional) */ | ||
| scope?: string; | ||
| /** Audience for the token (optional) */ | ||
| audience?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Token refresh result | ||
| */ | ||
| export type TokenRefreshResult = { | ||
| /** Access token */ | ||
| accessToken: string; | ||
| /** Token expiry time in seconds */ | ||
| expiresIn: number; | ||
| /** Token type (usually "Bearer") */ | ||
| tokenType: string; | ||
| /** Refresh token (if provided) */ | ||
| refreshToken?: string; | ||
| /** OAuth2 scope (if provided) */ | ||
| scope?: string; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Keep the client auth models in the canonical auth types module.
AuthConfig, OAuth2Config, and TokenRefreshResult recreate auth models in a second file. That brings back the split source of truth PR #892 removed. Please move these declarations to src/lib/types/authTypes.ts and re-export them here if the client entry needs them. Based on learnings, auth-related TypeScript types must live in src/lib/types/authTypes.ts, and the previous duplicate auth-types path was deleted in PR #892.
🤖 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 998 - 1052, The AuthConfig,
OAuth2Config, and TokenRefreshResult type declarations are duplicated here;
remove these local declarations and instead use the canonical auth types
module's definitions: delete the AuthConfig, OAuth2Config, and
TokenRefreshResult blocks and replace them with imports (and re-exports if
needed) pointing to the single source of truth so the client entry references
the same types (keep references to AuthConfig, OAuth2Config, and
TokenRefreshResult in the file but have them imported/re-exported from the
canonical auth types module).
Comment on lines
+134
to
+135
| const SERVER_URL = `http://localhost:${TEST_CONFIG.serverPort}`; | ||
|
|
There was a problem hiding this comment.
Compute SERVER_URL after parsing CLI args.
SERVER_URL is frozen with the default port at Lines 134-135, but Line 1216 mutates TEST_CONFIG.serverPort. With --port, the server starts on the new port while every client and SSE URL still points at 9200, so the advertised CLI override cannot work.
Also applies to: 1208-1217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/continuous-test-suite-client.ts` around lines 134 - 135, SERVER_URL is
computed too early using TEST_CONFIG.serverPort (const SERVER_URL =
`http://localhost:${TEST_CONFIG.serverPort}`) before CLI parsing mutates
TEST_CONFIG.serverPort; update the code to compute SERVER_URL after CLI args are
parsed (or replace the const with a function/getter that reads
TEST_CONFIG.serverPort at use-time) and ensure any derived URLs (SSE/client URL
constructions in the same file) use this late-computed value so the --port
override is respected.
murdore
force-pushed
the
feat/client-sdks-rebased
branch
from
March 23, 2026 18:33
b06897d to
8320297
Compare
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 |
murdore
force-pushed
the
feat/client-sdks-rebased
branch
from
March 23, 2026 18:46
8320297 to
5df61d0
Compare
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 |
murdore
force-pushed
the
feat/client-sdks-rebased
branch
from
March 23, 2026 19:04
5df61d0 to
b9f5506
Compare
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 |
murdore
force-pushed
the
feat/client-sdks-rebased
branch
from
March 23, 2026 19:13
b9f5506 to
fb307c1
Compare
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 |
…DK adapter Type-safe client libraries for accessing NeuroLink APIs from browser and Node.js applications. Includes HTTP client with middleware/interceptors, SSE and WebSocket streaming, React hooks (useChat, useAgent, useWorkflow, useVoice, useStream, useTools), Vercel AI SDK compatibility layer, OAuth2/JWT authentication, and comprehensive error handling. - Add src/lib/client/ with 10 implementation files - Add src/lib/types/clientTypes.ts with all client types in canonical location - Add ./client sub-path export in package.json - Add docs/features/client-sdk.md with full feature documentation - Add 4 examples (node-backend, react-chat, vercel-ai-sdk, websocket) - Add continuous test suite (13 tests, all passing) - Fix server SSE streaming to emit proper StreamEvent format with [DONE] - Add jsx: react-jsx to tsconfig for .tsx support
murdore
force-pushed
the
feat/client-sdks-rebased
branch
from
March 23, 2026 19:24
fb307c1 to
ff5badd
Compare
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
|
🎉 This PR is included in version 9.31.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
6 of 7 tasks
This branch had an error being 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
Changes
New files
src/lib/client/— 10 implementation files (httpClient, streamingClient, sseClient, wsClient, aiSdkAdapter, auth, errors, interceptors, reactHooks, index)src/lib/types/clientTypes.ts— all client types in canonicalsrc/lib/types/locationdocs/features/client-sdk.md— 614-line feature documentation with proper frontmatterexamples/client-sdks/— 4 examples (node-backend, react-chat, vercel-ai-sdk, websocket-realtime)test/continuous-test-suite-client.ts— 13-test continuous suite (all passing)Modified files
src/lib/index.ts— added client SDK exports with Client-prefixed error namessrc/lib/types/index.ts— added clientTypes selective exports with aliasessrc/lib/server/adapters/honoAdapter.ts— fixed SSE streaming to emit proper StreamEvent format with [DONE] terminationpackage.json— added./clientsub-path export,test:clientscript, React devDepstsconfig.json/tsconfig.cli.json— addedjsx: react-jsxfor .tsx supportdocs/features/index.md— added Client SDK entry to feature tableArchitecture decisions
src/lib/types/clientTypes.ts(canonical location, not in feature dir)Clientto avoid collisions with core error typesTest plan
npx tsc --noEmit --strict— 0 errorspnpm run test:client— 13/13 passing (health, generate, stream, middleware, auth, errors, JWT, AI SDK, SSE, error handling, React hooks, retry)Summary by CodeRabbit
New Features
generateText,streamText, andgenerateObject.Documentation
Tests